----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58853/#review173479 -----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java Line 685 (original), 689 (patched) <https://reviews.apache.org/r/58853/#comment246475> I see you have changed code to call this method. So it should not longer be labelled as a "test method". geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java Line 6202 (original), 6213 (patched) <https://reviews.apache.org/r/58853/#comment246476> This type of code looks like it could have a race condition because it checks getEventTracker() to see if it is non-null and then calls a method on it. It makes you wonder if a race condition could exist in which it becomes null. The only writer of the eventTracker field is "createEventTracker". It is implemented on both DistributedRegion and BucketRegion. I see no need for the BucketRegion impl since it is exactly the same as DistributedRegion and BucketRegion extends DistributedRegion. I think some refactoring should be done to make eventTracker a final field but it does not need to be done for this ticket. - Darrel Schneider On April 28, 2017, 1:17 p.m., Eric Shu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58853/ > ----------------------------------------------------------- > > (Updated April 28, 2017, 1:17 p.m.) > > > Review request for geode, anilkumar gingade, Darrel Schneider, and Lynn > Gallinat. > > > Bugs: GEODE-2847 > https://issues.apache.org/jira/browse/GEODE-2847 > > > Repository: geode > > > Description > ------- > > Get correct version tags from recordedBulkOpVersionTags in eventTracker. > Do not remove the recordedBulkOpVersionTags prematurely. > Add the unit test which would fail without the fixes. > > > Diffs > ----- > > geode-core/src/main/java/org/apache/geode/internal/cache/EventTracker.java > 2ddfdc4 > geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java > 8c061b0 > > geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/PutAllPRMessage.java > 27f5aa0 > > geode-core/src/main/java/org/apache/geode/internal/cache/partitioned/RemoveAllPRMessage.java > f4f6299 > > geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientProxyMembershipID.java > 2cbf63b > > geode-core/src/test/java/org/apache/geode/internal/cache/EventTrackerTest.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/58853/diff/1/ > > > Testing > ------- > > precheckin. > > > Thanks, > > Eric Shu > >
