chevaris commented on code in PR #1264:
URL: https://github.com/apache/curator/pull/1264#discussion_r2046278696
##########
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 have my doubts here what is best, because awaitTermination assures you
that tasks were executed, BUT the Javadoc does NOT confirm that the thread has
been destroyed, so we can only rely that will happen eventually
After closing the PersistentTTLNode:
- When executor is internal, probably best is a loop with small sleeps to
check that thread disappears. Notice also that CloseExecutorService is in
another package and extracting methods for testing requires them to be public
that I really do NOT like.
- When executor is provided, probably the best is to use awaitTermination
and after that sleep for instance 100 msecs and check that the thread is still
there
What do you think?
--
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
