Re: Review Request 51875: GEODE-1885: Removed call to check readiness (region check) after the offheap region entry is released.

[anilkumar gingade]

> 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.

[Darrel Schneider]

---
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)


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)


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)


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
> 
>

Review Request 51875: GEODE-1885: Removed call to check readiness (region check) after the offheap region entry is released.

[anilkumar gingade]

---
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