Re: Review Request 56002: GEODE-1672 LRU region values are not recovered during disk recovery.

2017-02-01 Thread Eric Shu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56002/#review163822
---


Ship it!




Ship It!

- Eric Shu


On Feb. 1, 2017, 1:54 a.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56002/
> ---
> 
> (Updated Feb. 1, 2017, 1:54 a.m.)
> 
> 
> Review request for geode, Darrel Schneider, Eric Shu, Ken Howe, and Swapnil 
> Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1672: out of memory during recovery with overflow regions (heap LRU 
> configured).
> 
> When recovering the persistent files, GEODE stores the values into temp maps 
> (for regions) using a background thread, as these maps are not actual 
> regions,  these are not considered/included for LRU eviction, which causes 
> the system to run OOM. 
> 
> The problem is fixed by skipping recovering the values for LRU regiuons.
> 
> A new system property ""disk.recoverLruValues" is added to support reading 
> values for lru regions.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java 
> 5affdb5 
>   geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 32d6e30 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskLruRegRecoveryJUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegRecoveryJUnitTest.java
>  6a5c338 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegionHelperFactory.java
>  fa68044 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegionProperties.java
>  19662bf 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/PersistentPartitionedRegionJUnitTest.java
>  b0170e1 
> 
> Diff: https://reviews.apache.org/r/56002/diff/
> 
> 
> Testing
> ---
> 
> Added new test. 
> Run existing disk tests.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 56002: GEODE-1672 LRU region values are not recovered during disk recovery.

2017-01-31 Thread anilkumar gingade 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56002/
---

(Updated Feb. 1, 2017, 1:54 a.m.)


Review request for geode, Darrel Schneider, Eric Shu, Ken Howe, and Swapnil 
Bawaskar.


Changes
---

Added additional test as per review comments.


Repository: geode


Description
---

GEODE-1672: out of memory during recovery with overflow regions (heap LRU 
configured).

When recovering the persistent files, GEODE stores the values into temp maps 
(for regions) using a background thread, as these maps are not actual regions,  
these are not considered/included for LRU eviction, which causes the system to 
run OOM. 

The problem is fixed by skipping recovering the values for LRU regiuons.

A new system property ""disk.recoverLruValues" is added to support reading 
values for lru regions.


Diffs (updated)
-

  geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java 
5affdb5 
  geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 32d6e30 
  
geode-core/src/test/java/org/apache/geode/internal/cache/DiskLruRegRecoveryJUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegRecoveryJUnitTest.java
 6a5c338 
  
geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegionHelperFactory.java
 fa68044 
  
geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegionProperties.java
 19662bf 
  
geode-core/src/test/java/org/apache/geode/internal/cache/PersistentPartitionedRegionJUnitTest.java
 b0170e1 

Diff: https://reviews.apache.org/r/56002/diff/


Testing
---

Added new test. 
Run existing disk tests.


Thanks,

anilkumar gingade

Re: Review Request 56002: GEODE-1672 LRU region values are not recovered during disk recovery.

2017-01-31 Thread anilkumar gingade 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56002/
---

(Updated Jan. 31, 2017, 11:17 p.m.)


Review request for geode, Darrel Schneider, Eric Shu, Ken Howe, and Swapnil 
Bawaskar.


Changes
---

Added diskstore offline check in "isLruValueRecoveryDisabled" condition. In 
case of offline recovery cases (export diskstore) the values are recovered to 
take the snapshot.


Repository: geode


Description
---

GEODE-1672: out of memory during recovery with overflow regions (heap LRU 
configured).

When recovering the persistent files, GEODE stores the values into temp maps 
(for regions) using a background thread, as these maps are not actual regions,  
these are not considered/included for LRU eviction, which causes the system to 
run OOM. 

The problem is fixed by skipping recovering the values for LRU regiuons.

A new system property ""disk.recoverLruValues" is added to support reading 
values for lru regions.


Diffs (updated)
-

  geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java 
5affdb5 
  geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 32d6e30 
  
geode-core/src/test/java/org/apache/geode/internal/cache/DiskLruRegRecoveryJUnitTest.java
 PRE-CREATION 
  
geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegRecoveryJUnitTest.java
 6a5c338 
  
geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegionHelperFactory.java
 fa68044 
  
geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegionProperties.java
 19662bf 
  
geode-core/src/test/java/org/apache/geode/internal/cache/PersistentPartitionedRegionJUnitTest.java
 b0170e1 

Diff: https://reviews.apache.org/r/56002/diff/


Testing
---

Added new test. 
Run existing disk tests.


Thanks,

anilkumar gingade

Re: Review Request 56002: GEODE-1672 LRU region values are not recovered during disk recovery.

2017-01-30 Thread Eric Shu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56002/#review163602
---




geode-core/src/test/java/org/apache/geode/internal/cache/PersistentPartitionedRegionJUnitTest.java
 (line 196)


You may want to add test cases for partitioned region so that setting 
DiskStoreImpl.RECOVER_LRU_VALUES_PROPERTY_NAME to true would preserve the old 
behavior.


- Eric Shu


On Jan. 28, 2017, 12:44 a.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56002/
> ---
> 
> (Updated Jan. 28, 2017, 12:44 a.m.)
> 
> 
> Review request for geode, Darrel Schneider, Eric Shu, Ken Howe, and Swapnil 
> Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1672: out of memory during recovery with overflow regions (heap LRU 
> configured).
> 
> When recovering the persistent files, GEODE stores the values into temp maps 
> (for regions) using a background thread, as these maps are not actual 
> regions,  these are not considered/included for LRU eviction, which causes 
> the system to run OOM. 
> 
> The problem is fixed by skipping recovering the values for LRU regiuons.
> 
> A new system property ""disk.recoverLruValues" is added to support reading 
> values for lru regions.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java 
> 5affdb5 
>   geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 32d6e30 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskLruRegRecoveryJUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegRecoveryJUnitTest.java
>  6a5c338 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegionHelperFactory.java
>  fa68044 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegionProperties.java
>  19662bf 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/PersistentPartitionedRegionJUnitTest.java
>  b0170e1 
> 
> Diff: https://reviews.apache.org/r/56002/diff/
> 
> 
> Testing
> ---
> 
> Added new test. 
> Run existing disk tests.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>

Re: Review Request 56002: GEODE-1672 LRU region values are not recovered during disk recovery.

2017-01-27 Thread anilkumar gingade 


> On Jan. 27, 2017, 12:23 a.m., Darrel Schneider wrote:
> > geode-core/src/test/java/org/apache/geode/internal/cache/DiskLruRegRecoveryJUnitTest.java,
> >  line 32
> > 
> >
> > also test PR regions

Added test for PR region.


> On Jan. 27, 2017, 12:23 a.m., Darrel Schneider wrote:
> > geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java,
> >  line 127
> > 
> >
> > I think you should add a comment describing how this sys prop changes 
> > the product behavior

Added comment for the new property.


- anilkumar


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56002/#review163210
---


On Jan. 27, 2017, 12:03 a.m., anilkumar gingade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56002/
> ---
> 
> (Updated Jan. 27, 2017, 12:03 a.m.)
> 
> 
> Review request for geode, Darrel Schneider, Eric Shu, Ken Howe, and Swapnil 
> Bawaskar.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-1672: out of memory during recovery with overflow regions (heap LRU 
> configured).
> 
> When recovering the persistent files, GEODE stores the values into temp maps 
> (for regions) using a background thread, as these maps are not actual 
> regions,  these are not considered/included for LRU eviction, which causes 
> the system to run OOM. 
> 
> The problem is fixed by skipping recovering the values for LRU regiuons.
> 
> A new system property ""disk.recoverLruValues" is added to support reading 
> values for lru regions.
> 
> 
> Diffs
> -
> 
>   geode-core/src/main/java/org/apache/geode/internal/cache/DiskStoreImpl.java 
> 5affdb5 
>   geode-core/src/main/java/org/apache/geode/internal/cache/Oplog.java 32d6e30 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskLruRegRecoveryJUnitTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegRecoveryJUnitTest.java
>  6a5c338 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegionHelperFactory.java
>  fa68044 
>   
> geode-core/src/test/java/org/apache/geode/internal/cache/DiskRegionProperties.java
>  19662bf 
> 
> Diff: https://reviews.apache.org/r/56002/diff/
> 
> 
> Testing
> ---
> 
> Added new test. 
> Run existing disk tests.
> 
> 
> Thanks,
> 
> anilkumar gingade
> 
>