----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58187/#review171087 -----------------------------------------------------------
Looks good. I left a few questions on the test, but the non-test code looks rockin'. Mostly I didn't understand how the CacheServerLauncher gets used. geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java Line 1288 (original) <https://reviews.apache.org/r/58187/#comment243948> No more JRockit! I'm hoping we've officially dropped support? geode-core/src/test/java/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.java Lines 30 (patched) <https://reviews.apache.org/r/58187/#comment243949> Could you replace this with a helpful description, perhaps? geode-core/src/test/java/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.java Lines 40 (patched) <https://reviews.apache.org/r/58187/#comment243954> Why is this necessary? ... wait, I'll guess. It's a distributed test, so it will need to get serialized to the other VMs in the system, but we don't need to actually check that its serialization is the same because it's the only time it will get sent, so we just set this to a bogus value? (this means that serialization doesn't have an internal numerical type registry but just checks `serialVersionUID` for failure?) geode-core/src/test/java/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.java Lines 75 (patched) <https://reviews.apache.org/r/58187/#comment243950> Does the `CacheServerLauncher` actually get used to create a Cache? geode-core/src/test/java/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.java Lines 89 (patched) <https://reviews.apache.org/r/58187/#comment243952> I'm trying to parse the use of an `AtomicBoolean` without Awaitility. looks like this is guaranteed to be called once the function returns, but not necessarily to have been called in this thread? geode-core/src/test/java/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.java Lines 93 (patched) <https://reviews.apache.org/r/58187/#comment243953> So Cache is a singleton except when it gets replaced by reconnection? Whoa. - Galen O'Sullivan On April 4, 2017, 8:46 p.m., Bruce Schuchardt wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58187/ > ----------------------------------------------------------- > > (Updated April 4, 2017, 8:46 p.m.) > > > Review request for geode, Galen O'Sullivan, Hitesh Khamesra, and Udo > Kohlmeyer. > > > Repository: geode > > > Description > ------- > > Gfsh command line parameters were put into ThreadLocals to make them > available to the XML parser. These are now held in non-thread-local > variables so that all threads, including the auto-reconnect thread, can see > them when building the cache. > > This diff also includes some minor refactoring in > org.apache.geode.internal.tcp. It primarily removes some > "this.owner.getConduit().doSomething" with "this.conduit.doSomething" and > removes dead code. > > > Diffs > ----- > > > extensions/geode-modules-tomcat8/src/test/java/org/apache/geode/modules/session/Tomcat8SessionsClientServerDUnitTest.java > e475f40339e294ad48b403db1afa7aa624d5e80c > geode-core/src/main/java/org/apache/geode/distributed/ServerLauncher.java > 9435bd8178248c4fa34f6867d15a7cdfdec06d6b > > geode-core/src/main/java/org/apache/geode/internal/cache/CacheServerLauncher.java > 760abd3d946281d474502a9af2e8fe823a011329 > > geode-core/src/main/java/org/apache/geode/internal/cache/xmlcache/CacheCreation.java > 1c3c93314841f5623c0e6387500af88f106328d9 > geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java > a0af24501a5081fe483160c5bf4fc7671545684e > geode-core/src/main/java/org/apache/geode/internal/tcp/ConnectionTable.java > 08a90096c0cf0348c0fa34b7ff8e18806810ab68 > > geode-core/src/main/java/org/apache/geode/internal/tcp/DirectReplySender.java > 3872ee98ec24fe95894c0def538e3752136b05c3 > geode-core/src/main/java/org/apache/geode/internal/tcp/MsgReader.java > fc5627112ac8dd456976139d3750fd951812879b > geode-core/src/main/java/org/apache/geode/internal/tcp/NIOMsgReader.java > 50f5faedba056ec4f57124aefd56477dc11c1cf6 > > geode-core/src/test/java/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.java > PRE-CREATION > > geode-core/src/test/resources/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.xml > PRE-CREATION > > > Diff: https://reviews.apache.org/r/58187/diff/1/ > > > Testing > ------- > > new test, precheckin > > > Thanks, > > Bruce Schuchardt > >
