You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Darrel Schneider <ds...@pivotal.io> on 2017/05/01 17:47:07 UTC
Re: Review Request 58853: GEODE-2847: Get correct version tags for
retried bulk operations
-----------------------------------------------------------
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
>
>