> On April 5, 2017, 11:24 p.m., Galen O'Sullivan wrote: > > geode-core/src/main/java/org/apache/geode/internal/tcp/Connection.java > > Line 1288 (original) > > <https://reviews.apache.org/r/58187/diff/1/?file=1684571#file1684571line1295> > > > > No more JRockit! I'm hoping we've officially dropped support?
I'll remove that comment but I'm not sure we can remove the code it's attached to. > On April 5, 2017, 11:24 p.m., Galen O'Sullivan wrote: > > geode-core/src/test/java/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.java > > Lines 30 (patched) > > <https://reviews.apache.org/r/58187/diff/1/?file=1684576#file1684576line30> > > > > Could you replace this with a helpful description, perhaps? I'll replace it with this: /** * This test exercises auto-reconnect functionality when there is a cache-server * that was started by gfsh but was configured both by gfsh and a cache.xml * file. The JIRA ticket for this is GEODE-2732. */ > On April 5, 2017, 11:24 p.m., Galen O'Sullivan wrote: > > geode-core/src/test/java/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.java > > Lines 40 (patched) > > <https://reviews.apache.org/r/58187/diff/1/?file=1684576#file1684576line40> > > > > 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?) I copied another test and gutted most of it to create this test class. We set the serialVersionUID to 1 in lots of test classes to get rid of findbugs warnings. We don't need to worry about the on-wire format of the test changing since all JVMs are using the same classpath so the value of this variable doesn't matter. > On April 5, 2017, 11:24 p.m., Galen O'Sullivan wrote: > > geode-core/src/test/java/org/apache/geode/cache30/ReconnectWithCacheXMLDUnitTest.java > > Lines 75 (patched) > > <https://reviews.apache.org/r/58187/diff/1/?file=1684576#file1684576line75> > > > > Does the `CacheServerLauncher` actually get used to create a Cache? These variables are used in various places in GemFire to discover gfsh "start server" settings. I'm told they're going to be removed during a refactoring in the future. - Bruce ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58187/#review171087 ----------------------------------------------------------- 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 > >
