You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by anilkumar gingade <ag...@pivotal.io> on 2016/06/09 19:07:06 UTC
Re: Review Request 48445: GEODE-11: Adding stats for lucene documents, files,
and chunks
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48445/#review136863
-----------------------------------------------------------
Fix it, then Ship it!
Ship It!
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java (line 91)
<https://reviews.apache.org/r/48445/#comment201941>
Since we are creating the regions here; do we need to call region size?
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java (line 108)
<https://reviews.apache.org/r/48445/#comment201944>
Will it be a good idea to move it to parent class (LuceneIndexIml)? the stats will be same across multiple impletmentations (replicated, partition, others), right?
geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexStats.java (line 33)
<https://reviews.apache.org/r/48445/#comment201949>
Sorry if i am too late to comment on this...
I think we should make stats public...We should have public APIs on LuceneIndex that returns LuceneIndexStats (define interface with helper methods)...
User could use this in their applications, if needed...
- anilkumar gingade
On June 8, 2016, 9:44 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48445/
> -----------------------------------------------------------
>
> (Updated June 8, 2016, 9:44 p.m.)
>
>
> Review request for geode, anilkumar gingade, Jason Huynh, and nabarun nag.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Using the new stat suppliers added with GEODE-1494 to sample useful
> lucene statistics:
> * Number of documents
> * Number of files
> * Number of chunks
> * Number of bytes
>
>
> Diffs
> -----
>
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java 8fe5fac7313d647435c4931dcb27d99160f5a49b
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexImpl.java c75148acc83ef3823ca475a10862ac0a9ce0dc44
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexStats.java 8dd34a4fea7b6fa83cf4b0e62bd9fd170c7c8ced
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystemStats.java 1f609fd53487c6c6070a11c5cff039ab1900c322
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java 88631b842b50d0b3d6367198e3e1c44be958ea4e
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java 07780ca06a297851c8ff4cb498a09f726a0785d5
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexStatsJUnitTest.java 3e6d44c2967973cc5e0aee219f79ef5d11ef1b9c
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplJUnitTest.java 6004623b22ab36e3b24f9b7de3423fe776f07586
>
> Diff: https://reviews.apache.org/r/48445/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>
Re: Review Request 48445: GEODE-11: Adding stats for lucene documents, files,
and chunks
Posted by Dan Smith <ds...@pivotal.io>.
> On June 9, 2016, 7:07 p.m., anilkumar gingade wrote:
> > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java, line 91
> > <https://reviews.apache.org/r/48445/diff/1/?file=1411627#file1411627line91>
> >
> > Since we are creating the regions here; do we need to call region size?
What this is doing here is installing a *callback* that will be called every sample interval to compute the region size.
> On June 9, 2016, 7:07 p.m., anilkumar gingade wrote:
> > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java, line 108
> > <https://reviews.apache.org/r/48445/diff/1/?file=1411627#file1411627line108>
> >
> > Will it be a good idea to move it to parent class (LuceneIndexIml)? the stats will be same across multiple impletmentations (replicated, partition, others), right?
Makes sense. I'll move them up.
> On June 9, 2016, 7:07 p.m., anilkumar gingade wrote:
> > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexStats.java, line 33
> > <https://reviews.apache.org/r/48445/diff/1/?file=1411629#file1411629line33>
> >
> > Sorry if i am too late to comment on this...
> >
> > I think we should make stats public...We should have public APIs on LuceneIndex that returns LuceneIndexStats (define interface with helper methods)...
> > User could use this in their applications, if needed...
I was just following the pattern for other stats in geode - we create an internal class for managing the stat. But maybe creating public interfaces that expose getters for our stats is a good idea. Let's discuss and consider that for a later commit.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48445/#review136863
-----------------------------------------------------------
On June 8, 2016, 9:44 p.m., Dan Smith wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48445/
> -----------------------------------------------------------
>
> (Updated June 8, 2016, 9:44 p.m.)
>
>
> Review request for geode, anilkumar gingade, Jason Huynh, and nabarun nag.
>
>
> Repository: geode
>
>
> Description
> -------
>
> Using the new stat suppliers added with GEODE-1494 to sample useful
> lucene statistics:
> * Number of documents
> * Number of files
> * Number of chunks
> * Number of bytes
>
>
> Diffs
> -----
>
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java 8fe5fac7313d647435c4931dcb27d99160f5a49b
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexImpl.java c75148acc83ef3823ca475a10862ac0a9ce0dc44
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexStats.java 8dd34a4fea7b6fa83cf4b0e62bd9fd170c7c8ced
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/filesystem/FileSystemStats.java 1f609fd53487c6c6070a11c5cff039ab1900c322
> geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java 88631b842b50d0b3d6367198e3e1c44be958ea4e
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/LuceneIndexMaintenanceIntegrationTest.java 07780ca06a297851c8ff4cb498a09f726a0785d5
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexStatsJUnitTest.java 3e6d44c2967973cc5e0aee219f79ef5d11ef1b9c
> geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplJUnitTest.java 6004623b22ab36e3b24f9b7de3423fe776f07586
>
> Diff: https://reviews.apache.org/r/48445/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Dan Smith
>
>