----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59404/#review175825 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java Lines 288 (patched) <https://reviews.apache.org/r/59404/#comment249163> To avoid a race condition that results in an NPE do this: rp = this.createRegionReplyProcessor; if (rp != null) { rp.getRemoteEventStates().put(member, eventStates); } geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java Lines 301 (patched) <https://reviews.apache.org/r/59404/#comment249166> Instead of exposing the entire private Map by having a getRemoteEventStates method, just add a getEventState(InternalDistributedMember) method and use that here. geode-core/src/main/java/org/apache/geode/internal/cache/CreateRegionProcessor.java Lines 100 (patched) <https://reviews.apache.org/r/59404/#comment249159> instead of casting to LocalRegion it would be better to add "registerCreateRegionReplyProcessor" to CacheDistributionAdvisee. Add it as a "default" method that does nothing. Then you only need to override it on BucketRegion. No need to add a noop to LocalRegion. geode-core/src/main/java/org/apache/geode/internal/cache/CreateRegionProcessor.java Line 202 (original), 204 (patched) <https://reviews.apache.org/r/59404/#comment249158> make this final geode-core/src/main/java/org/apache/geode/internal/cache/CreateRegionProcessor.java Line 250 (original), 252 (patched) <https://reviews.apache.org/r/59404/#comment249160> I see no need for you to go to "lr" for this. Can't you record it locally in your own 'remoteEventStates' map? This allows you to completely get rid of the recordPendingEventState. Also get rid of "lr.hasEventTracker". If you get a non-null eventState back just stick it in your local remoteEventStates map. geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java Lines 542 (patched) <https://reviews.apache.org/r/59404/#comment249161> This comment is not needed. You method name makes this clear. geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java Lines 543 (patched) <https://reviews.apache.org/r/59404/#comment249162> Move this call down to line 554 right before the call of endGetInitialImage geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java Lines 544 (patched) <https://reviews.apache.org/r/59404/#comment249165> Since "this.region" is a DistributedRegion it would be best for this method to start on it instead of LocalRegion. - Darrel Schneider On May 23, 2017, 10:55 a.m., Eric Shu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59404/ > ----------------------------------------------------------- > > (Updated May 23, 2017, 10:55 a.m.) > > > Review request for geode, anilkumar gingade, Darrel Schneider, and Lynn > Gallinat. > > > Bugs: GEODE-2939 > https://issues.apache.org/jira/browse/GEODE-2939 > > > Repository: geode > > > Description > ------- > > Event tracker initilization for bucket region is delayed until after GII, and > will be initialized from the GII provider. > > > Diffs > ----- > > geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegion.java > 886d678 > > geode-core/src/main/java/org/apache/geode/internal/cache/CreateRegionProcessor.java > c1d1e77 > geode-core/src/main/java/org/apache/geode/internal/cache/EventTracker.java > 2c86aed > > geode-core/src/main/java/org/apache/geode/internal/cache/InitialImageOperation.java > fb5f0cf > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java > 8e7ec68 > > geode-core/src/test/java/org/apache/geode/internal/cache/EventTrackerDUnitTest.java > 3faf41f > > > Diff: https://reviews.apache.org/r/59404/diff/2/ > > > Testing > ------- > > precheckin. > > > Thanks, > > Eric Shu > >
