> On Aug. 26, 2016, 9:27 p.m., Darrel Schneider wrote: > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/partitioned/PersistentColocatedPartitionedRegionDUnitTest.java, > > line 713 > > <https://reviews.apache.org/r/51465/diff/1/?file=1487003#file1487003line713> > > > > This is overkill for this test. Your grand-parent > > (JUnit4CacheTestCache) closes the cache on each jvm. I think you only need > > to close them early when you have an async thread stuck.
Two tests failed to call get(), getResult() or join() on all of their invokeAsync threads. One of those, testMissingColocatedParentPRWherePRConfigExists, really did have a thread that never terminated. Calls have beebn added for the threads where they were missing. > On Aug. 26, 2016, 9:27 p.m., Darrel Schneider wrote: > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/partitioned/PersistentColocatedPartitionedRegionDUnitTest.java, > > line 753 > > <https://reviews.apache.org/r/51465/diff/1/?file=1487003#file1487003line753> > > > > I think in general it would be better to have the closeCache and > > async0.join in a finally block after you have done the assertEquals. > > > > Also make clear that you expect async0 to block until closeCache is > > called. This test had a thread on vm0 that never terminated. A finally block has been added to create valid region configurations on vm1 after the expceted exception we received. When the vm1 regions are recreated the operations in the thread on vm0 should complete. Calls have been added to make sure the threads terminate and the caches close. > On Aug. 26, 2016, 9:27 p.m., Darrel Schneider wrote: > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/partitioned/PersistentColocatedPartitionedRegionDUnitTest.java, > > line 914 > > <https://reviews.apache.org/r/51465/diff/1/?file=1487003#file1487003line914> > > > > Another way to unstick these async0 guys that are waiting for vm1 to do > > something (like create all its regions) would be to have the test create > > those regions and confirm that we stop logging warnings in vm1 and that vm0 > > completes region creation. This particular test did not have a stuck thread. The indicated line has been remnoved as there was already an async0.get() before the assert. - Ken ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51465/#review147025 ----------------------------------------------------------- On Aug. 26, 2016, 8:36 p.m., Ken Howe wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51465/ > ----------------------------------------------------------- > > (Updated Aug. 26, 2016, 8:36 p.m.) > > > Review request for geode, anilkumar gingade, Darrel Schneider, and Kirk Lund. > > > Repository: geode > > > Description > ------- > > GEODE-1128: Cleaned up some of the colocation logging tests > > Ensure that the asyncInvocation threads terminate. Failure of a thread to > terminate on one of the tests is apparently responsible for at least 1 CI > failure. > > > Diffs > ----- > > > geode-core/src/test/java/com/gemstone/gemfire/internal/cache/partitioned/PersistentColocatedPartitionedRegionDUnitTest.java > b701b70f94f0a8410ea71dd7fb77f586ba456f3f > > Diff: https://reviews.apache.org/r/51465/diff/ > > > Testing > ------- > > precheckin in progress > > > Thanks, > > Ken Howe > >
