You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by anilkumar gingade <ag...@pivotal.io> on 2016/09/14 00:22:03 UTC
Review Request 51875: GEODE-1885: Removed call to check readiness
(region check) after the offheap region entry is released.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51875/
-----------------------------------------------------------
Review request for geode, Darrel Schneider, Eric Shu, Ken Howe, and Swapnil Bawaskar.
Repository: geode
Description
-------
GEODE-1885: Missing subsctiption event with Offheap partitioned region during bucket rebalance.
During the trasaction commit on redundant bucket region, if the bucket region is moved, the call-back logic (to deliver subscription events) were not invoked due to check-readiness call with offheap region. The check-readiness throws exception, if the region is not found, which causes the code to return early without sending the subscription events.
In this scenario, calling check-readiness was not needed...
Diffs
-----
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractDiskRegionEntry.java 41cd110
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionEntry.java 5778a82
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionMap.java 81e4d9f
geode-core/src/test/java/com/gemstone/gemfire/internal/cache/ClientServerTransactionDUnitTest.java 08953d5
Diff: https://reviews.apache.org/r/51875/diff/
Testing
-------
Reproduced the missing create event with the submitted test. And verified with the fix.
pre-checkin.
Thanks,
anilkumar gingade
Re: Review Request 51875: GEODE-1885: Removed call to check readiness
(region check) after the offheap region entry is released.
Posted by anilkumar gingade <ag...@pivotal.io>.
> On Sept. 14, 2016, 11:20 p.m., Darrel Schneider wrote:
> >
Spoke to Darrel, he is fine with the changes as is...
- anilkumar
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51875/#review148993
-----------------------------------------------------------
On Sept. 14, 2016, 12:22 a.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51875/
> -----------------------------------------------------------
>
> (Updated Sept. 14, 2016, 12:22 a.m.)
>
>
> Review request for geode, Darrel Schneider, Eric Shu, Ken Howe, and Swapnil Bawaskar.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1885: Missing subsctiption event with Offheap partitioned region during bucket rebalance.
>
> During the trasaction commit on redundant bucket region, if the bucket region is moved, the call-back logic (to deliver subscription events) were not invoked due to check-readiness call with offheap region. The check-readiness throws exception, if the region is not found, which causes the code to return early without sending the subscription events.
>
> In this scenario, calling check-readiness was not needed...
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractDiskRegionEntry.java 41cd110
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionEntry.java 5778a82
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionMap.java 81e4d9f
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/ClientServerTransactionDUnitTest.java 08953d5
>
> Diff: https://reviews.apache.org/r/51875/diff/
>
>
> Testing
> -------
>
> Reproduced the missing create event with the submitted test. And verified with the fix.
> pre-checkin.
>
>
> Thanks,
>
> anilkumar gingade
>
>
Re: Review Request 51875: GEODE-1885: Removed call to check readiness
(region check) after the offheap region entry is released.
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51875/#review148993
-----------------------------------------------------------
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionEntry.java (line 443)
<https://reviews.apache.org/r/51875/#comment216522>
We only call this method after calling _setValue.
I think it would be better if we moved the call of this method to _setValue. We would need to pass RegionEntryContext to _setValue but that seems pretty easy.
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionEntry.java (line 451)
<https://reviews.apache.org/r/51875/#comment216521>
It would be nice if RegionEntryContext has isThisRegionBeingClosedOrDestroyed (and drop "ThisRegion" from the name). Then you can get rid of the LocalRegion instanceof and just call context.isBeingClsoedOrDestroyed().
geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionEntry.java (line 456)
<https://reviews.apache.org/r/51875/#comment216523>
Seems like it would be better for this method to return false for AbstractRegionEntry and then override it in the OffHeapRegionEntry subclasses. Seeing an instanceof OffHeapRegionEntry in AbstractRegionEntry is bad.
- Darrel Schneider
On Sept. 13, 2016, 5:22 p.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51875/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2016, 5:22 p.m.)
>
>
> Review request for geode, Darrel Schneider, Eric Shu, Ken Howe, and Swapnil Bawaskar.
>
>
> Repository: geode
>
>
> Description
> -------
>
> GEODE-1885: Missing subsctiption event with Offheap partitioned region during bucket rebalance.
>
> During the trasaction commit on redundant bucket region, if the bucket region is moved, the call-back logic (to deliver subscription events) were not invoked due to check-readiness call with offheap region. The check-readiness throws exception, if the region is not found, which causes the code to return early without sending the subscription events.
>
> In this scenario, calling check-readiness was not needed...
>
>
> Diffs
> -----
>
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractDiskRegionEntry.java 41cd110
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionEntry.java 5778a82
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/AbstractRegionMap.java 81e4d9f
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/ClientServerTransactionDUnitTest.java 08953d5
>
> Diff: https://reviews.apache.org/r/51875/diff/
>
>
> Testing
> -------
>
> Reproduced the missing create event with the submitted test. And verified with the fix.
> pre-checkin.
>
>
> Thanks,
>
> anilkumar gingade
>
>