You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/06/02 19:25:13 UTC

[GitHub] [geode] kirklund commented on a diff in pull request #7697: GEODE-10304: locator thread should not exit after reconnecting

kirklund commented on code in PR #7697:
URL: https://github.com/apache/geode/pull/7697#discussion_r888303887


##########
geode-core/src/distributedTest/java/org/apache/geode/cache30/ReconnectDUnitTest.java:
##########
@@ -320,143 +304,128 @@ public void doTestReconnectOnForcedDisconnect(final boolean createInAppToo) thro
     Invoke
         .invokeInEveryVM(() -> DistributedTestUtils.deleteLocatorStateFile(locPort, secondLocPort));
 
-
     final String xmlFileLoc = (new File(".")).getAbsolutePath();
 
-    SerializableCallable create1 =
-        new SerializableCallable("Create Cache and Regions from cache.xml") {
-          @Override
-          public Object call() throws CacheException {
-            locatorPort = locPort;
-            Properties props = getDistributedSystemProperties();
-            props.put(CACHE_XML_FILE, xmlFileLoc + fileSeparator + "MyDisconnect-cache.xml");
-            props.put(MAX_WAIT_TIME_RECONNECT, "1000");
-            cache = (InternalCache) new CacheFactory(props).create();
-            Region myRegion = cache.getRegion("root" + SEPARATOR + "myRegion");
-            ReconnectDUnitTest.savedSystem = cache.getDistributedSystem();
-            myRegion.put("MyKey1", "MyValue1");
-            return savedSystem.getDistributedMember();
-          }
-        };
+    SerializableCallableIF<DistributedMember> create1 = () -> {
+      locatorPort = locPort;
+      Properties props = getDistributedSystemProperties();
+      props.put(CACHE_XML_FILE, xmlFileLoc + fileSeparator + "MyDisconnect-cache.xml");
+      props.put(MAX_WAIT_TIME_RECONNECT, "1000");
+      cache = (InternalCache) new CacheFactory(props).create();
+      Region myRegion = cache.getRegion("root" + SEPARATOR + "myRegion");
+      ReconnectDUnitTest.savedSystem = cache.getDistributedSystem();
+      myRegion.put("MyKey1", "MyValue1");
+      return savedSystem.getDistributedMember();
+    };
 
-    SerializableCallable create2 =
-        new SerializableCallable("Create Cache and Regions from cache.xml") {
+    SerializableCallableIF<DistributedMember> create2 = () -> {
+      locatorPort = locPort;
+      final Properties props = getDistributedSystemProperties();
+      props.put(CACHE_XML_FILE, xmlFileLoc + fileSeparator + "MyDisconnect-cache.xml");
+      props.put(MAX_WAIT_TIME_RECONNECT, "5000");
+      props.put(START_LOCATOR, "localhost[" + secondLocPort + "]");
+      props.put(LOCATORS, props.get(LOCATORS) + ",localhost[" + secondLocPort + "]");
+      getSystem(props);
+      cache = getCache();
+      ReconnectDUnitTest.savedSystem = cache.getDistributedSystem();
+      Region myRegion = cache.getRegion("root" + SEPARATOR + "myRegion");
+      myRegion.put("Mykey2", "MyValue2");
+      assertNotNull(myRegion.get("MyKey1"));
+      if (createInAppToo) {
+        Thread recreateCacheThread = new Thread("ReconnectDUnitTest.createInAppThread") {
           @Override
-          public Object call() throws CacheException {
-            locatorPort = locPort;
-            final Properties props = getDistributedSystemProperties();
-            props.put(CACHE_XML_FILE, xmlFileLoc + fileSeparator + "MyDisconnect-cache.xml");
-            props.put(MAX_WAIT_TIME_RECONNECT, "5000");
-            props.put(START_LOCATOR, "localhost[" + secondLocPort + "]");
-            props.put(LOCATORS, props.get(LOCATORS) + ",localhost[" + secondLocPort + "]");
-            getSystem(props);
-            cache = getCache();
-            ReconnectDUnitTest.savedSystem = cache.getDistributedSystem();
-            Region myRegion = cache.getRegion("root" + SEPARATOR + "myRegion");
-            myRegion.put("Mykey2", "MyValue2");
-            assertNotNull(myRegion.get("MyKey1"));
-            if (createInAppToo) {
-              Thread recreateCacheThread = new Thread("ReconnectDUnitTest.createInAppThread") {
-                @Override
-                public void run() {
-                  while (!cache.isClosed()) {
-                    Wait.pause(100);
-                  }
-                  try {
-                    cache = (InternalCache) new CacheFactory(props).create();
-                    System.err.println(
-                        "testReconnectCollidesWithApplication failed - application thread was able to create a cache");
-                  } catch (IllegalStateException cacheExists) {
-                    // expected
-                  }
-                }
-              };
-              recreateCacheThread.setDaemon(true);
-              recreateCacheThread.start();
+          public void run() {
+            while (!cache.isClosed()) {
+              Wait.pause(100);
+            }
+            try {
+              cache = (InternalCache) new CacheFactory(props).create();
+              System.err.println(
+                  "testReconnectCollidesWithApplication failed - application thread was able to create a cache");
+            } catch (IllegalStateException cacheExists) {
+              // expected

Review Comment:
   If `cache = (InternalCache) new CacheFactory(props).create();` is supposed to throw `IllegalStateException`, then that `System.err.println` should change to `fail("testReconnectCollidesWithApplication failed - application thread was able to create a cache");` or you could change the whole try-catch block to use AssertJ `catchThrowable` with appropriate assertion.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org