> On May 2, 2016, 11:34 p.m., Jianxia Chen wrote: > > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalDistributedSystem.java, > > line 2748 > > <https://reviews.apache.org/r/46850/diff/3/?file=1368991#file1368991line2748> > > > > Why this one does not need a lock? i.e. why not using > > isReconnectCancelled()
That is a good question. Looking at at the use of reconnectCancelled in this class, it is not consistently accessed under a sync on reconnectCancelledLock and there aren't any cases where the sync is used for notification so I think reconnectCancelled can just be made a volatile variable. I'll address that in another commit. - Bruce ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46850/#review131415 ----------------------------------------------------------- On May 2, 2016, 6:23 p.m., Bruce Schuchardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46850/ > ----------------------------------------------------------- > > (Updated May 2, 2016, 6:23 p.m.) > > > Review request for geode, Hitesh Khamesra, Jianxia Chen, and Udo Kohlmeyer. > > > Bugs: GEODE-1329 > https://issues.apache.org/jira/browse/GEODE-1329 > > > Repository: geode > > > Description > ------- > > Users would like auto-reconnect to ensure that a node retries even if it has > been immediately kicked out of the distributed system. > InternalDistributedSystem.reconnect() now includes cache-creation in its > retry loop and, should the cache fail to start due to a CancelException it > will shut down and try again. > > While creating a new test in ReconnectDUnitTest I found problems with the > other tests in that class that I fixed. Notably the method > getDistributedSystemProperties() wasn't returning the correct properties for > a lot of test cases because the dsProperties variable wasn't being set by > them and they were using the getCache() method. This was causing the current > distributed system to be destroyed and a new one created with different > properties than the test wanted. That was causing periodic test failures. > > > Diffs > ----- > > > geode-core/src/main/java/com/gemstone/gemfire/distributed/DistributedSystem.java > 1de675d6d5492c0f418f000fff2b2a47cc0704b2 > > geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/InternalDistributedSystem.java > 3ef8e80e031672b26f34c9896d76e72fab2b9d48 > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/DistributedRegion.java > 226d914c2f28677ca39cf8f0bcda2d2783cd2076 > > geode-core/src/main/java/com/gemstone/gemfire/internal/cache/GemFireCacheImpl.java > c4774668e14834f15f1de7b029e5ef2ad1fe20e8 > > geode-core/src/test/java/com/gemstone/gemfire/cache30/ReconnectDUnitTest.java > a4ba33def4745fbdebac1eb0ce90b75246197ec5 > > Diff: https://reviews.apache.org/r/46850/diff/ > > > Testing > ------- > > New unit test. Precheckin is running. Multihost network partition testing. > > > Thanks, > > Bruce Schuchardt > >
