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




geode-core/src/main/java/com/gemstone/gemfire/internal/cache/PartitionedRegion.java
 (line 7490)
<https://reviews.apache.org/r/50715/#comment212491>

    A race exists here.
    After you test "missingColocatedRegionLogger it may become null and then 
you will get an NPE on line 7493.
    So change this to something like this:
      TYPE regionLogger = missingColocatedRegionLogger;
      if (regionLogger != null) {
        return regionLogger.getMissingChildRegions();
      }
      return Collections.emptyList();


- Darrel Schneider


On Aug. 18, 2016, 10:51 a.m., Ken Howe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50715/
> -----------------------------------------------------------
> 
> (Updated Aug. 18, 2016, 10:51 a.m.)
> 
> 
> Review request for geode, Darrel Schneider, Eric Shu, Scott Jewell, Kirk 
> Lund, and Dan Smith.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> Test for missing parent regions on child region creates and throw 
> IllegalStateException
> 
> Log warnings about missing colocated child regions.
> 
> Create Unit and DUnit tests for new exceptions and warnings
> 
> There are two cases of missing colocated regions,
> 1) The 'parent' region hasn't been created when a region specifies it in the 
> 'colocated-with' attribute
> 2) A persistent child region does not exist.
> 
> For (1), this condition can be determined in 
> ColocationHelper.getColocatedRegion(). The core product currently does not 
> test for this which results in an NPE being thrown without any logging to 
> indicate the reason. There are two variations of this state.
> 
> 1a) When starting a region with non-null 'colocated'with', a reference to the 
> parent region configuration is obtained through the configuration root. When 
> the reference obtained is null (the region doesn't exist in the root 
> configuration) the NPE ends up getting thrown. The fix for this is to 
> immediately throw an IllegalStateException with a message to note the missing 
> colocated-with region.
> 
> 1b) The parent region configuration may exist in the root configuration (the 
> parent PR has been created on another member) but does not exist on the local 
> member. In this case the null comes about when obtaining the local region 
> (PartitionedRegion.getPRFromId()). The fix here is the same as (1a) - throw 
> an IllegalStateException.
> 
> For (2), missing child regions: This state will always exist for an 
> indeterminate period because the parent is always created before the child 
> region. There currently isn't any indication in the logs of this condition, 
> even it if persists indefinitely, other than a failure to recover the PR's 
> persistent data. The fix for this is starting a logging thread (similar to 
> the RedundancyLogger) when a child region is found missing. The condition 
> will be periodically logged (set initially to 30 secs) until the region is 
> created. There is a delay before the first log message to allow time for the 
> normal sequencing of region creations.
> 
> 
> Diffs
> -----
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ColocationHelper.java
>  012a77f 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/ColocationLogger.java
>  PRE-CREATION 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/PartitionRegionConfig.java
>  6d7c1ca 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/PartitionedRegion.java
>  9ac95a1 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/i18n/LocalizedStrings.java
>  443fe78 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/ColocationHelperTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/partitioned/PersistentColocatedPartitionedRegionDUnitTest.java
>  d8b3514 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/cache/partitioned/PersistentPartitionedRegionTestBase.java
>  692378c 
> 
> Diff: https://reviews.apache.org/r/50715/diff/
> 
> 
> Testing
> -------
> 
> precheckin in progress
> 
> 
> Thanks,
> 
> Ken Howe
> 
>

Reply via email to