chevaris commented on code in PR #1264:
URL: https://github.com/apache/curator/pull/1264#discussion_r2047192585


##########
curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentTtlNode.java:
##########
@@ -237,4 +241,88 @@ void touch() {
             }
         }
     }
+
+    @Test
+    public void testInternalExecutorClose() throws Exception {
+        final String mainPath = "/parent/main";
+        final String touchPath = ZKPaths.makePath(mainPath, 
PersistentTtlNode.DEFAULT_CHILD_NODE_NAME);
+        final CountDownLatch touchCreatedLatch = new CountDownLatch(1);
+        try (CuratorFramework client =
+                CuratorFrameworkFactory.newClient(server.getConnectString(), 
new RetryOneTime(1))) {
+            client.start();
+            assertTrue(client.blockUntilConnected(1, TimeUnit.SECONDS));
+            try (PersistentWatcher watcher = new PersistentWatcher(client, 
mainPath, true)) {
+                final Watcher listener = event -> {
+                    final String path = event.getPath();
+                    if (touchPath.equals(path)) {
+                        touchCreatedLatch.countDown();
+                    }
+                };
+                watcher.getListenable().addListener(listener);
+                watcher.start();
+                final AtomicLong executorThreadId = new AtomicLong();
+                try (PersistentTtlNode node = new PersistentTtlNode(client, 
mainPath, ttlMs, new byte[0])) {
+                    node.start();
+                    assertTrue(touchCreatedLatch.await(5 * ttlMs, 
TimeUnit.MILLISECONDS));
+                    node.getCloseableScheduledExecutorService()
+                            .submit(() ->
+                                    
executorThreadId.set(Thread.currentThread().getId()));
+                    assertFalse(getThreadsWithIdAndName(executorThreadId, 
PersistentTtlNode.TOUCH_THREAD_NAME)
+                            .isEmpty());
+                }
+                Thread.sleep(10L);

Review Comment:
   I realized (doing the code) that awaitTermination requires that 
shutdown/shutdownNow has been previously invoked , otherwise will NOT work as 
expected . So when executor is provided, after PersistentTTLNode has been 
closed I will wait for 100ms and check that executor there is still there
   
   Maybe looking for the thread using the threadId and the name is a bit too 
much, when it is enough to check if the executor isShutdown or NOT.
   
   I will provide commit that is able to check if excutor is shutdown in both 
cases, and also the thread checks and you provide comments. Now I am more in 
favour to remove all the code that looks for the thread from stacks traces, BUT 
just give me your opinion there



-- 
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: commits-unsubscr...@curator.apache.org

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

Reply via email to