----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55956/#review163019 -----------------------------------------------------------
Hey, Looks like this is a big improvement on the reliability of this stuff. I have some comments, but if we need to tackle this incrementally thats fine. geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/AbstractPartitionedRepositoryManager.java (line 122) <https://reviews.apache.org/r/55956/#comment234399> Why is this doing a second get? geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/AbstractPartitionedRepositoryManager.java (line 132) <https://reviews.apache.org/r/55956/#comment234405> Maybe this should be remove instead of get? geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java (line 64) <https://reviews.apache.org/r/55956/#comment234408> This may somehow need to be synchronized with the cleanup code, to handle race conditions where we lose the primary status in the middle of this code. Maybe add all of these tasks to an a thread pool with a single thread? If you do that, make sure the thread pool is fair and garuantees the tasks will be executed in order. geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LucenePrimaryBucketListener.java (line 58) <https://reviews.apache.org/r/55956/#comment234402> Should probably be logged or something? geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/repository/IndexRepositoryImpl.java (line 167) <https://reviews.apache.org/r/55956/#comment234403> It would probably be better to pass in the lock service and the key to avoid casting and having this class know about a concrete bucket. geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/repository/IndexRepositoryImpl.java (line 174) <https://reviews.apache.org/r/55956/#comment234415> Maybe the unlock should be the last thing that happens, so that another member can't create a writer until after this member closes the writer. - Dan Smith On Jan. 25, 2017, 8:46 p.m., Jason Huynh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55956/ > ----------------------------------------------------------- > > (Updated Jan. 25, 2017, 8:46 p.m.) > > > Review request for geode, Barry Oglesby, nabarun nag, Dan Smith, and xiaojian > zhou. > > > Repository: geode > > > Description > ------- > > Added afterSecondary callback to partition listener to allow cleaning up of > the index repo when the bucket losses primary > Added lock prior to creating the bucket indexes to prevent multiple index > writers from being available at a time > Changed single point of lucene index creation, no longer creating on the fly > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/cache/partition/PartitionListener.java > a534e50 > geode-core/src/main/java/org/apache/geode/internal/cache/BucketAdvisor.java > 7b79bfb > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/AbstractPartitionedRepositoryManager.java > aa29e1b > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/IndexRepositoryFactory.java > c73d64a > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventListener.java > f2c7c8f > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LucenePrimaryBucketListener.java > d17b5f2 > > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/repository/IndexRepositoryImpl.java > 4e86eb5 > > Diff: https://reviews.apache.org/r/55956/diff/ > > > Testing > ------- > > > Thanks, > > Jason Huynh > >
