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/04/12 18:55:24 UTC
Review Request 58397: check off-heap limit during disk recovery
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58397/
-----------------------------------------------------------
Review request for geode, anilkumar gingade, Eric Shu, and Lynn Gallinat.
Bugs: GEODE-2097
https://issues.apache.org/jira/browse/GEODE-2097
Repository: geode
Description
-------
During disk recovery, the code now checks the offheap LRU limit instead of the heap LRU limit for offheap regions.
A unit test has been added that without this fix would run out of offheap memory during recovery.
Diffs
-----
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java 328ff35b940239bfbcf817d144d97f886506b33a
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java eaababaf6be6041c72156a338c69fa81f10db4c5
geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java d0aacc2d95fa38b1bc9faf68de795af5ef3d9090
geode-core/src/main/java/org/apache/geode/internal/cache/PlaceHolderDiskRegion.java db01162aeeba8113c208565d683e71a41d0f6d00
geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java 92c7b6f8e90a3382e7ce00064581d69f9706fcd3
geode-core/src/main/java/org/apache/geode/internal/cache/lru/EnableLRU.java 6aaf8cc914c3b8c2e308446930bbd3215c9f58ca
geode-core/src/main/java/org/apache/geode/internal/cache/lru/HeapLRUCapacityController.java 5e86ce87b99fc4e8793461e881853c141fdfec1f
geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUCapacityController.java 3596a07df407960c3558057ac33e92c89bd5ba22
geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUMapCallbacks.java 27a4ec019ae3b5fd29a26ffc451d0d9feb75f903
geode-core/src/main/java/org/apache/geode/internal/cache/lru/MemLRUCapacityController.java 2c2e8ec1a805b7485e6e59858a12122fa8d88de1
geode-core/src/test/java/org/apache/geode/internal/cache/lru/LRUClockJUnitTest.java 8095d5ac483be061199623a78b63618697e09d9d
geode-core/src/test/java/org/apache/geode/internal/offheap/OffHeapLRURecoveryRegressionTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/58397/diff/1/
Testing
-------
precheckin
Thanks,
Darrel Schneider
Re: Review Request 58397: check off-heap limit during disk recovery
Posted by Darrel Schneider <ds...@pivotal.io>.
> On April 12, 2017, 2:50 p.m., Eric Shu wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUCapacityController.java
> > Lines 294 (patched)
> > <https://reviews.apache.org/r/58397/diff/1/?file=1691066#file1691066line294>
> >
> > lruStatistics or stats passed in?
Good catch. The intent here was to use the parameter; not the instance variable.
I think in most (perhaps all cases) they end up being the same thing.
- Darrel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58397/#review171801
-----------------------------------------------------------
On April 12, 2017, 11:55 a.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58397/
> -----------------------------------------------------------
>
> (Updated April 12, 2017, 11:55 a.m.)
>
>
> Review request for geode, anilkumar gingade, Eric Shu, and Lynn Gallinat.
>
>
> Bugs: GEODE-2097
> https://issues.apache.org/jira/browse/GEODE-2097
>
>
> Repository: geode
>
>
> Description
> -------
>
> During disk recovery, the code now checks the offheap LRU limit instead of the heap LRU limit for offheap regions.
> A unit test has been added that without this fix would run out of offheap memory during recovery.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java 328ff35b940239bfbcf817d144d97f886506b33a
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java eaababaf6be6041c72156a338c69fa81f10db4c5
> geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java d0aacc2d95fa38b1bc9faf68de795af5ef3d9090
> geode-core/src/main/java/org/apache/geode/internal/cache/PlaceHolderDiskRegion.java db01162aeeba8113c208565d683e71a41d0f6d00
> geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java 92c7b6f8e90a3382e7ce00064581d69f9706fcd3
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/EnableLRU.java 6aaf8cc914c3b8c2e308446930bbd3215c9f58ca
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/HeapLRUCapacityController.java 5e86ce87b99fc4e8793461e881853c141fdfec1f
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUCapacityController.java 3596a07df407960c3558057ac33e92c89bd5ba22
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUMapCallbacks.java 27a4ec019ae3b5fd29a26ffc451d0d9feb75f903
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/MemLRUCapacityController.java 2c2e8ec1a805b7485e6e59858a12122fa8d88de1
> geode-core/src/test/java/org/apache/geode/internal/cache/lru/LRUClockJUnitTest.java 8095d5ac483be061199623a78b63618697e09d9d
> geode-core/src/test/java/org/apache/geode/internal/offheap/OffHeapLRURecoveryRegressionTest.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58397/diff/1/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Darrel Schneider
>
>
Re: Review Request 58397: check off-heap limit during disk recovery
Posted by Eric Shu <es...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58397/#review171801
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUCapacityController.java
Lines 294 (patched)
<https://reviews.apache.org/r/58397/#comment244804>
lruStatistics or stats passed in?
- Eric Shu
On April 12, 2017, 6:55 p.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58397/
> -----------------------------------------------------------
>
> (Updated April 12, 2017, 6:55 p.m.)
>
>
> Review request for geode, anilkumar gingade, Eric Shu, and Lynn Gallinat.
>
>
> Bugs: GEODE-2097
> https://issues.apache.org/jira/browse/GEODE-2097
>
>
> Repository: geode
>
>
> Description
> -------
>
> During disk recovery, the code now checks the offheap LRU limit instead of the heap LRU limit for offheap regions.
> A unit test has been added that without this fix would run out of offheap memory during recovery.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java 328ff35b940239bfbcf817d144d97f886506b33a
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java eaababaf6be6041c72156a338c69fa81f10db4c5
> geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java d0aacc2d95fa38b1bc9faf68de795af5ef3d9090
> geode-core/src/main/java/org/apache/geode/internal/cache/PlaceHolderDiskRegion.java db01162aeeba8113c208565d683e71a41d0f6d00
> geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java 92c7b6f8e90a3382e7ce00064581d69f9706fcd3
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/EnableLRU.java 6aaf8cc914c3b8c2e308446930bbd3215c9f58ca
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/HeapLRUCapacityController.java 5e86ce87b99fc4e8793461e881853c141fdfec1f
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUCapacityController.java 3596a07df407960c3558057ac33e92c89bd5ba22
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUMapCallbacks.java 27a4ec019ae3b5fd29a26ffc451d0d9feb75f903
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/MemLRUCapacityController.java 2c2e8ec1a805b7485e6e59858a12122fa8d88de1
> geode-core/src/test/java/org/apache/geode/internal/cache/lru/LRUClockJUnitTest.java 8095d5ac483be061199623a78b63618697e09d9d
> geode-core/src/test/java/org/apache/geode/internal/offheap/OffHeapLRURecoveryRegressionTest.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58397/diff/1/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Darrel Schneider
>
>
Re: Review Request 58397: check off-heap limit during disk recovery
Posted by Eric Shu <es...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58397/#review171896
-----------------------------------------------------------
Ship it!
Ship It!
- Eric Shu
On April 12, 2017, 11:42 p.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58397/
> -----------------------------------------------------------
>
> (Updated April 12, 2017, 11:42 p.m.)
>
>
> Review request for geode, anilkumar gingade, Eric Shu, and Lynn Gallinat.
>
>
> Bugs: GEODE-2097
> https://issues.apache.org/jira/browse/GEODE-2097
>
>
> Repository: geode
>
>
> Description
> -------
>
> During disk recovery, the code now checks the offheap LRU limit instead of the heap LRU limit for offheap regions.
> A unit test has been added that without this fix would run out of offheap memory during recovery.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java 328ff35b940239bfbcf817d144d97f886506b33a
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java eaababaf6be6041c72156a338c69fa81f10db4c5
> geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java d0aacc2d95fa38b1bc9faf68de795af5ef3d9090
> geode-core/src/main/java/org/apache/geode/internal/cache/PlaceHolderDiskRegion.java db01162aeeba8113c208565d683e71a41d0f6d00
> geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java 92c7b6f8e90a3382e7ce00064581d69f9706fcd3
> geode-core/src/main/java/org/apache/geode/internal/cache/control/HeapMemoryMonitor.java afc9a23aafba2c1f993b30546f24cd2b5d6a0126
> geode-core/src/main/java/org/apache/geode/internal/cache/control/InternalResourceManager.java d16cd98bbb3eefc785a22f41441c1490060d0781
> geode-core/src/main/java/org/apache/geode/internal/cache/control/MemoryMonitor.java PRE-CREATION
> geode-core/src/main/java/org/apache/geode/internal/cache/control/OffHeapMemoryMonitor.java 414036747a5062ab556b60cdb58c5ce490312c78
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/EnableLRU.java 6aaf8cc914c3b8c2e308446930bbd3215c9f58ca
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/HeapLRUCapacityController.java 5e86ce87b99fc4e8793461e881853c141fdfec1f
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUCapacityController.java 3596a07df407960c3558057ac33e92c89bd5ba22
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUMapCallbacks.java 27a4ec019ae3b5fd29a26ffc451d0d9feb75f903
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/MemLRUCapacityController.java 2c2e8ec1a805b7485e6e59858a12122fa8d88de1
> geode-core/src/test/java/org/apache/geode/internal/cache/lru/LRUClockJUnitTest.java 8095d5ac483be061199623a78b63618697e09d9d
> geode-core/src/test/java/org/apache/geode/internal/offheap/OffHeapLRURecoveryRegressionTest.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58397/diff/2/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Darrel Schneider
>
>
Re: Review Request 58397: check off-heap limit during disk recovery
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58397/
-----------------------------------------------------------
(Updated April 12, 2017, 4:42 p.m.)
Review request for geode, anilkumar gingade, Eric Shu, and Lynn Gallinat.
Changes
-------
added MemoryMonitor interface based on Anil's feedback
change method to use lruStatistics (the param) instead of stats (the inst var).
Bugs: GEODE-2097
https://issues.apache.org/jira/browse/GEODE-2097
Repository: geode
Description
-------
During disk recovery, the code now checks the offheap LRU limit instead of the heap LRU limit for offheap regions.
A unit test has been added that without this fix would run out of offheap memory during recovery.
Diffs (updated)
-----
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java 328ff35b940239bfbcf817d144d97f886506b33a
geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java eaababaf6be6041c72156a338c69fa81f10db4c5
geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java d0aacc2d95fa38b1bc9faf68de795af5ef3d9090
geode-core/src/main/java/org/apache/geode/internal/cache/PlaceHolderDiskRegion.java db01162aeeba8113c208565d683e71a41d0f6d00
geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java 92c7b6f8e90a3382e7ce00064581d69f9706fcd3
geode-core/src/main/java/org/apache/geode/internal/cache/control/HeapMemoryMonitor.java afc9a23aafba2c1f993b30546f24cd2b5d6a0126
geode-core/src/main/java/org/apache/geode/internal/cache/control/InternalResourceManager.java d16cd98bbb3eefc785a22f41441c1490060d0781
geode-core/src/main/java/org/apache/geode/internal/cache/control/MemoryMonitor.java PRE-CREATION
geode-core/src/main/java/org/apache/geode/internal/cache/control/OffHeapMemoryMonitor.java 414036747a5062ab556b60cdb58c5ce490312c78
geode-core/src/main/java/org/apache/geode/internal/cache/lru/EnableLRU.java 6aaf8cc914c3b8c2e308446930bbd3215c9f58ca
geode-core/src/main/java/org/apache/geode/internal/cache/lru/HeapLRUCapacityController.java 5e86ce87b99fc4e8793461e881853c141fdfec1f
geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUCapacityController.java 3596a07df407960c3558057ac33e92c89bd5ba22
geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUMapCallbacks.java 27a4ec019ae3b5fd29a26ffc451d0d9feb75f903
geode-core/src/main/java/org/apache/geode/internal/cache/lru/MemLRUCapacityController.java 2c2e8ec1a805b7485e6e59858a12122fa8d88de1
geode-core/src/test/java/org/apache/geode/internal/cache/lru/LRUClockJUnitTest.java 8095d5ac483be061199623a78b63618697e09d9d
geode-core/src/test/java/org/apache/geode/internal/offheap/OffHeapLRURecoveryRegressionTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/58397/diff/2/
Changes: https://reviews.apache.org/r/58397/diff/1-2/
Testing
-------
precheckin
Thanks,
Darrel Schneider
Re: Review Request 58397: check off-heap limit during disk recovery
Posted by Darrel Schneider <ds...@pivotal.io>.
> On April 12, 2017, 3:44 p.m., anilkumar gingade wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/lru/HeapLRUCapacityController.java
> > Line 283 (original), 274 (patched)
> > <https://reviews.apache.org/r/58397/diff/1/?file=1691065#file1691065line284>
> >
> > Is it more readable if it is:
> >
> > if (offHeap) {
> > use offheap monitor
> > } else {
> > heap monitor
> > }
I agree. I will do some refactoring and get rid of this if/else
- Darrel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58397/#review171803
-----------------------------------------------------------
On April 12, 2017, 11:55 a.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58397/
> -----------------------------------------------------------
>
> (Updated April 12, 2017, 11:55 a.m.)
>
>
> Review request for geode, anilkumar gingade, Eric Shu, and Lynn Gallinat.
>
>
> Bugs: GEODE-2097
> https://issues.apache.org/jira/browse/GEODE-2097
>
>
> Repository: geode
>
>
> Description
> -------
>
> During disk recovery, the code now checks the offheap LRU limit instead of the heap LRU limit for offheap regions.
> A unit test has been added that without this fix would run out of offheap memory during recovery.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java 328ff35b940239bfbcf817d144d97f886506b33a
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java eaababaf6be6041c72156a338c69fa81f10db4c5
> geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java d0aacc2d95fa38b1bc9faf68de795af5ef3d9090
> geode-core/src/main/java/org/apache/geode/internal/cache/PlaceHolderDiskRegion.java db01162aeeba8113c208565d683e71a41d0f6d00
> geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java 92c7b6f8e90a3382e7ce00064581d69f9706fcd3
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/EnableLRU.java 6aaf8cc914c3b8c2e308446930bbd3215c9f58ca
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/HeapLRUCapacityController.java 5e86ce87b99fc4e8793461e881853c141fdfec1f
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUCapacityController.java 3596a07df407960c3558057ac33e92c89bd5ba22
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUMapCallbacks.java 27a4ec019ae3b5fd29a26ffc451d0d9feb75f903
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/MemLRUCapacityController.java 2c2e8ec1a805b7485e6e59858a12122fa8d88de1
> geode-core/src/test/java/org/apache/geode/internal/cache/lru/LRUClockJUnitTest.java 8095d5ac483be061199623a78b63618697e09d9d
> geode-core/src/test/java/org/apache/geode/internal/offheap/OffHeapLRURecoveryRegressionTest.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58397/diff/1/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Darrel Schneider
>
>
Re: Review Request 58397: check off-heap limit during disk recovery
Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58397/#review171803
-----------------------------------------------------------
geode-core/src/main/java/org/apache/geode/internal/cache/lru/HeapLRUCapacityController.java
Line 283 (original), 274 (patched)
<https://reviews.apache.org/r/58397/#comment244807>
Is it more readable if it is:
if (offHeap) {
use offheap monitor
} else {
heap monitor
}
- anilkumar gingade
On April 12, 2017, 6:55 p.m., Darrel Schneider wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58397/
> -----------------------------------------------------------
>
> (Updated April 12, 2017, 6:55 p.m.)
>
>
> Review request for geode, anilkumar gingade, Eric Shu, and Lynn Gallinat.
>
>
> Bugs: GEODE-2097
> https://issues.apache.org/jira/browse/GEODE-2097
>
>
> Repository: geode
>
>
> Description
> -------
>
> During disk recovery, the code now checks the offheap LRU limit instead of the heap LRU limit for offheap regions.
> A unit test has been added that without this fix would run out of offheap memory during recovery.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractLRURegionMap.java 328ff35b940239bfbcf817d144d97f886506b33a
> geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java eaababaf6be6041c72156a338c69fa81f10db4c5
> geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java d0aacc2d95fa38b1bc9faf68de795af5ef3d9090
> geode-core/src/main/java/org/apache/geode/internal/cache/PlaceHolderDiskRegion.java db01162aeeba8113c208565d683e71a41d0f6d00
> geode-core/src/main/java/org/apache/geode/internal/cache/ProxyRegionMap.java 92c7b6f8e90a3382e7ce00064581d69f9706fcd3
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/EnableLRU.java 6aaf8cc914c3b8c2e308446930bbd3215c9f58ca
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/HeapLRUCapacityController.java 5e86ce87b99fc4e8793461e881853c141fdfec1f
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUCapacityController.java 3596a07df407960c3558057ac33e92c89bd5ba22
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/LRUMapCallbacks.java 27a4ec019ae3b5fd29a26ffc451d0d9feb75f903
> geode-core/src/main/java/org/apache/geode/internal/cache/lru/MemLRUCapacityController.java 2c2e8ec1a805b7485e6e59858a12122fa8d88de1
> geode-core/src/test/java/org/apache/geode/internal/cache/lru/LRUClockJUnitTest.java 8095d5ac483be061199623a78b63618697e09d9d
> geode-core/src/test/java/org/apache/geode/internal/offheap/OffHeapLRURecoveryRegressionTest.java PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/58397/diff/1/
>
>
> Testing
> -------
>
> precheckin
>
>
> Thanks,
>
> Darrel Schneider
>
>