> On May 26, 2016, 6:53 p.m., xiaojian zhou wrote: > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/PartitionedRepositoryManager.java, > > line 92 > > <https://reviews.apache.org/r/47912/diff/1/?file=1395637#file1395637line92> > > > > I'm concerning about "stats" showed up in many constructor methods. > > > > The stats object should be keeped in LuceneIndexImpl object, then we > > use index object to reference it in other classes, such as > > RepositoryManager.
Hmm, this might be a matter of personal taste. I like having the dependencies explicitly listed in the contructor, because it makes it easier to write tests. If we pass one big index object everywhere, tests of PartitionRepositoryManager and IndexRepository have to mock the index and then mock the stats as well. - Dan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47912/#review135044 ----------------------------------------------------------- On May 26, 2016, 6:26 p.m., Dan Smith wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47912/ > ----------------------------------------------------------- > > (Updated May 26, 2016, 6:26 p.m.) > > > Review request for geode, anilkumar gingade, Barry Oglesby, Jason Huynh, > nabarun nag, and xiaojian zhou. > > > Repository: geode > > > Description > ------- > > Adding some initial stats for lucene indexes tracking updates and queries. > > I haven't yet figured out how to update the documents stat, but I figured I > would get the easy stats checked in first. We're tracking updates, commits, > and query stats with this change. > > > Diffs > ----- > > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexForPartitionedRegion.java > d22ca4a196df3b1a457b56c92da694bdbf792cc2 > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexImpl.java > c165085c83b38930bb970ac8f8e3f3fd8aa8808f > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexStats.java > PRE-CREATION > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/PartitionedRepositoryManager.java > 57b8862d91cb0b825de27d67ff594c984fc8ca55 > > geode-lucene/src/main/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImpl.java > 065cc6a5c2b140a2fb383362d9e8d1316e903e8a > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexRecoveryHAIntegrationTest.java > 194f3c7adfa32d0f2c8e9f18a27a3217d488560c > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/LuceneIndexStatsJUnitTest.java > PRE-CREATION > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/PartitionedRepositoryManagerJUnitTest.java > 4532d1651845c429bfa2b9f2f4eebf9e6e2bbdb2 > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/distributed/DistributedScoringJUnitTest.java > 7bcc7619198c360d0ec3a5d887bb0b2b4a483d8d > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplJUnitTest.java > 53c41616042dfe71febe7bc192c74f26f3938203 > > geode-lucene/src/test/java/com/gemstone/gemfire/cache/lucene/internal/repository/IndexRepositoryImplPerformanceTest.java > cec76ba0533093c88d653879323acba4e1901c9e > > Diff: https://reviews.apache.org/r/47912/diff/ > > > Testing > ------- > > > Thanks, > > Dan Smith > >
