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 2017/07/18 01:20:41 UTC
Review Request 60935: GEODE-3115 Added changes to check for persistent
region during pdx type registry.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60935/
-----------------------------------------------------------
Review request for geode, Darrel Schneider, Eric Shu, Lynn Gallinat, and Nick Reich.
Repository: geode
Description
-------
The pdx registry needs to be persisted to use pdx with persistent region. Currently while creating pdx the system checks, to see if there is a disk store with data instead of looking for persistent region in the system. In cases where persistent region is created and dropped, the system doesn't allow creating pdx witout setting pdx persistence.
Diffs
-----
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 2dda38c
geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java 4c229db
geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java 61f91a0
geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java 065255b
geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionTestBase.java 45c6120
geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java ef15cd5
Diff: https://reviews.apache.org/r/60935/diff/1/
Testing
-------
Added new test.
Precheckin started.
Thanks,
anilkumar gingade
Re: Review Request 60935: GEODE-3115 Added changes to check for
persistent region during pdx type registry.
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60935/#review181283
-----------------------------------------------------------
Ship it!
Ship It!
- Darrel Schneider
On July 21, 2017, 12:18 p.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60935/
> -----------------------------------------------------------
>
> (Updated July 21, 2017, 12:18 p.m.)
>
>
> Review request for geode, Darrel Schneider, Eric Shu, Lynn Gallinat, and Nick Reich.
>
>
> Repository: geode
>
>
> Description
> -------
>
> The pdx registry needs to be persisted to use pdx with persistent region. Currently while creating pdx the system checks, to see if there is a disk store with data instead of looking for persistent region in the system. In cases where persistent region is created and dropped, the system doesn't allow creating pdx witout setting pdx persistence.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java de5fd88
> geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java aed439c
> geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java db5f7ca
> geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java 065255b
> geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java ef15cd5
>
>
> Diff: https://reviews.apache.org/r/60935/diff/2/
>
>
> Testing
> -------
>
> Added new test.
> Precheckin started.
>
>
> Thanks,
>
> anilkumar gingade
>
>
Re: Review Request 60935: GEODE-3115 Added changes to check for
persistent region during pdx type registry.
Posted by Nick Reich <nr...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60935/#review181270
-----------------------------------------------------------
Ship it!
Ship It!
- Nick Reich
On July 21, 2017, 7:18 p.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60935/
> -----------------------------------------------------------
>
> (Updated July 21, 2017, 7:18 p.m.)
>
>
> Review request for geode, Darrel Schneider, Eric Shu, Lynn Gallinat, and Nick Reich.
>
>
> Repository: geode
>
>
> Description
> -------
>
> The pdx registry needs to be persisted to use pdx with persistent region. Currently while creating pdx the system checks, to see if there is a disk store with data instead of looking for persistent region in the system. In cases where persistent region is created and dropped, the system doesn't allow creating pdx witout setting pdx persistence.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java de5fd88
> geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java aed439c
> geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java db5f7ca
> geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java 065255b
> geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java ef15cd5
>
>
> Diff: https://reviews.apache.org/r/60935/diff/2/
>
>
> Testing
> -------
>
> Added new test.
> Precheckin started.
>
>
> Thanks,
>
> anilkumar gingade
>
>
Re: Review Request 60935: GEODE-3115 Added changes to check for
persistent region during pdx type registry.
Posted by anilkumar gingade <ag...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60935/
-----------------------------------------------------------
(Updated July 21, 2017, 7:18 p.m.)
Review request for geode, Darrel Schneider, Eric Shu, Lynn Gallinat, and Nick Reich.
Changes
-------
Changes as suggested in review comments.
Repository: geode
Description
-------
The pdx registry needs to be persisted to use pdx with persistent region. Currently while creating pdx the system checks, to see if there is a disk store with data instead of looking for persistent region in the system. In cases where persistent region is created and dropped, the system doesn't allow creating pdx witout setting pdx persistence.
Diffs (updated)
-----
geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java de5fd88
geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java aed439c
geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java db5f7ca
geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java 065255b
geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java ef15cd5
Diff: https://reviews.apache.org/r/60935/diff/2/
Changes: https://reviews.apache.org/r/60935/diff/1-2/
Testing
-------
Added new test.
Precheckin started.
Thanks,
anilkumar gingade
Re: Review Request 60935: GEODE-3115 Added changes to check for
persistent region during pdx type registry.
Posted by Darrel Schneider <ds...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60935/#review180828
-----------------------------------------------------------
Fix it, then Ship it!
fix then ship
geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java
Line 532 (original), 531 (patched)
<https://reviews.apache.org/r/60935/#comment256114>
A minor nit: change "hasPersistentRegions" to "hasPersistentRegion" since we only check that is has at least one persistent region.
geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionTestBase.java
Line 391 (original), 391 (patched)
<https://reviews.apache.org/r/60935/#comment256113>
You should remove the change to this file before checking in just to reduce the number of changed files.
- Darrel Schneider
On July 17, 2017, 6:20 p.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60935/
> -----------------------------------------------------------
>
> (Updated July 17, 2017, 6:20 p.m.)
>
>
> Review request for geode, Darrel Schneider, Eric Shu, Lynn Gallinat, and Nick Reich.
>
>
> Repository: geode
>
>
> Description
> -------
>
> The pdx registry needs to be persisted to use pdx with persistent region. Currently while creating pdx the system checks, to see if there is a disk store with data instead of looking for persistent region in the system. In cases where persistent region is created and dropped, the system doesn't allow creating pdx witout setting pdx persistence.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 2dda38c
> geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java 4c229db
> geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java 61f91a0
> geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java 065255b
> geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionTestBase.java 45c6120
> geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java ef15cd5
>
>
> Diff: https://reviews.apache.org/r/60935/diff/1/
>
>
> Testing
> -------
>
> Added new test.
> Precheckin started.
>
>
> Thanks,
>
> anilkumar gingade
>
>
Re: Review Request 60935: GEODE-3115 Added changes to check for
persistent region during pdx type registry.
Posted by Nick Reich <nr...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60935/#review180818
-----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java
Lines 370 (patched)
<https://reviews.apache.org/r/60935/#comment256088>
Duplicate code as above test: consider refactoring out a helper method.
geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java
Lines 377 (patched)
<https://reviews.apache.org/r/60935/#comment256090>
leftover println from debugging?
geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java
Line 335 (original), 382 (patched)
<https://reviews.apache.org/r/60935/#comment256099>
JUnit has a good way to test for exceptions using rules (added in 4, so the try catch idiom was the only option back in JUnit 3): http://junit.org/junit4/javadoc/4.12/org/junit/rules/ExpectedException.html
- Nick Reich
On July 18, 2017, 1:20 a.m., anilkumar gingade wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60935/
> -----------------------------------------------------------
>
> (Updated July 18, 2017, 1:20 a.m.)
>
>
> Review request for geode, Darrel Schneider, Eric Shu, Lynn Gallinat, and Nick Reich.
>
>
> Repository: geode
>
>
> Description
> -------
>
> The pdx registry needs to be persisted to use pdx with persistent region. Currently while creating pdx the system checks, to see if there is a disk store with data instead of looking for persistent region in the system. In cases where persistent region is created and dropped, the system doesn't allow creating pdx witout setting pdx persistence.
>
>
> Diffs
> -----
>
> geode-core/src/main/java/org/apache/geode/internal/cache/GemFireCacheImpl.java 2dda38c
> geode-core/src/main/java/org/apache/geode/internal/cache/InternalCache.java 4c229db
> geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java 61f91a0
> geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java 065255b
> geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/PersistentPartitionedRegionTestBase.java 45c6120
> geode-core/src/test/java/org/apache/geode/pdx/PdxAttributesJUnitTest.java ef15cd5
>
>
> Diff: https://reviews.apache.org/r/60935/diff/1/
>
>
> Testing
> -------
>
> Added new test.
> Precheckin started.
>
>
> Thanks,
>
> anilkumar gingade
>
>