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