This is an automated email from the ASF dual-hosted git repository.
kezhuw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/curator.git
The following commit(s) were added to refs/heads/master by this push:
new 3f631ac86 Fix PersistentTtlNode not deleted if touch node is never
created (#1260)
3f631ac86 is described below
commit 3f631ac866d0dd119be3724a7c73b4487abc8dfa
Author: chevaris <[email protected]>
AuthorDate: Tue Apr 8 14:15:45 2025 +0200
Fix PersistentTtlNode not deleted if touch node is never created (#1260)
Closes #1258.
See also CURATOR-545, ZOOKEEPER-3546(apache/zookeeper#1138).
---
.../framework/recipes/nodes/PersistentTtlNode.java | 65 +++++++++++-----------
.../recipes/nodes/TestPersistentTtlNode.java | 50 +++++++++++++++++
2 files changed, 83 insertions(+), 32 deletions(-)
diff --git
a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentTtlNode.java
b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentTtlNode.java
index 2b2ba561a..33e1608a2 100644
---
a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentTtlNode.java
+++
b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentTtlNode.java
@@ -19,6 +19,7 @@
package org.apache.curator.framework.recipes.nodes;
+import com.google.common.annotations.VisibleForTesting;
import java.io.Closeable;
import java.io.IOException;
import java.util.Objects;
@@ -37,7 +38,7 @@ import org.slf4j.LoggerFactory;
/**
* <p>
- * Manages a {@link PersistentNode} that uses {@link
CreateMode#CONTAINER}. Asynchronously
+ * Manages a {@link PersistentNode} that uses {@link
CreateMode#PERSISTENT_WITH_TTL}. Asynchronously
* it creates or updates a child on the persistent node that is marked
with a provided TTL.
* </p>
*
@@ -46,7 +47,7 @@ import org.slf4j.LoggerFactory;
* a method of having the parent node deleted if the TTL expires. i.e. if
the process
* that is running the PersistentTtlNode crashes and the TTL elapses,
first the child node
* will be deleted due to the TTL expiration and then the parent node will
be deleted as it's
- * a container node with no children.
+ * a TTL node with no children.
* </p>
*
* <p>
@@ -159,46 +160,46 @@ public class PersistentTtlNode implements Closeable {
this.client = Objects.requireNonNull(client, "client cannot be null");
this.ttlMs = ttlMs;
this.touchScheduleFactor = touchScheduleFactor;
- node = new PersistentNode(client, CreateMode.CONTAINER, false, path,
initData, useParentCreation) {
- @Override
- protected void deleteNode() {
- // NOP
- }
- };
+ node =
+ new PersistentNode(
+ client, CreateMode.PERSISTENT_WITH_TTL, false, path,
initData, ttlMs, useParentCreation) {
+ @Override
+ protected void deleteNode() {
+ // NOP
+ }
+ };
this.executorService = Objects.requireNonNull(executorService,
"executorService cannot be null");
childPath = ZKPaths.makePath(Objects.requireNonNull(path, "path cannot
be null"), childNodeName);
}
+ @VisibleForTesting
+ void touch() {
+ try {
+ try {
+ client.setData().forPath(childPath);
+ } catch (KeeperException.NoNodeException e) {
+ client.create()
+ .orSetData()
+ .withTtl(ttlMs)
+ .withMode(CreateMode.PERSISTENT_WITH_TTL)
+ .forPath(childPath);
+ }
+ } catch (KeeperException.NoNodeException ignore) {
+ // ignore
+ } catch (Exception e) {
+ if (!ThreadUtils.checkInterrupted(e)) {
+ log.debug("Could not touch child node", e);
+ }
+ }
+ }
+
/**
* You must call start() to initiate the persistent ttl node
*/
public void start() {
node.start();
-
- Runnable touchTask = new Runnable() {
- @Override
- public void run() {
- try {
- try {
- client.setData().forPath(childPath);
- } catch (KeeperException.NoNodeException e) {
- client.create()
- .orSetData()
- .withTtl(ttlMs)
- .withMode(CreateMode.PERSISTENT_WITH_TTL)
- .forPath(childPath);
- }
- } catch (KeeperException.NoNodeException ignore) {
- // ignore
- } catch (Exception e) {
- if (!ThreadUtils.checkInterrupted(e)) {
- log.debug("Could not touch child node", e);
- }
- }
- }
- };
Future<?> future = executorService.scheduleAtFixedRate(
- touchTask, ttlMs / touchScheduleFactor, ttlMs /
touchScheduleFactor, TimeUnit.MILLISECONDS);
+ this::touch, ttlMs / touchScheduleFactor, ttlMs /
touchScheduleFactor, TimeUnit.MILLISECONDS);
futureRef.set(future);
}
diff --git
a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentTtlNode.java
b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentTtlNode.java
index ddc9f3975..b98ada445 100644
---
a/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentTtlNode.java
+++
b/curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentTtlNode.java
@@ -22,22 +22,28 @@ package org.apache.curator.framework.recipes.nodes;
import static
org.apache.curator.framework.recipes.cache.PathChildrenCache.StartMode.BUILD_INITIAL_CACHE;
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.curator.framework.CuratorFramework;
import org.apache.curator.framework.CuratorFrameworkFactory;
import org.apache.curator.framework.recipes.cache.PathChildrenCache;
import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent;
import org.apache.curator.framework.recipes.cache.PathChildrenCacheListener;
+import org.apache.curator.framework.recipes.watch.PersistentWatcher;
import org.apache.curator.retry.RetryOneTime;
import org.apache.curator.test.Timing;
import org.apache.curator.test.compatibility.CuratorTestBase;
import org.apache.curator.test.compatibility.Timing2;
import org.apache.curator.utils.ZKPaths;
+import org.apache.zookeeper.Watcher;
+import org.apache.zookeeper.Watcher.Event.EventType;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
@@ -187,4 +193,48 @@ public class TestPersistentTtlNode extends CuratorTestBase
{
assertNull(client.checkExists().forPath("/test"));
}
}
+
+ @Test
+ public void testTouchNodeNotCreated() throws Exception {
+ final String mainPath = "/parent/main";
+ final String touchPath = ZKPaths.makePath(mainPath,
PersistentTtlNode.DEFAULT_CHILD_NODE_NAME);
+ final long testTtlMs = 500L;
+ final CountDownLatch mainCreatedLatch = new CountDownLatch(1);
+ final CountDownLatch mainDeletedLatch = new CountDownLatch(1);
+ final AtomicBoolean touchCreated = new AtomicBoolean();
+ 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 (mainPath.equals(path)) {
+ final EventType type = event.getType();
+ if (EventType.NodeCreated.equals(type)) {
+ mainCreatedLatch.countDown();
+ } else if (EventType.NodeDeleted.equals(type)) {
+ mainDeletedLatch.countDown();
+ }
+ } else if (touchPath.equals(path)) {
+ touchCreated.set(true);
+ }
+ };
+ watcher.getListenable().addListener(listener);
+ watcher.start();
+ try (PersistentTtlNode node = new PersistentTtlNode(client,
mainPath, testTtlMs, new byte[0]) {
+ @Override
+ void touch() {
+ // NOP
+ }
+ }) {
+ node.start();
+ assertTrue(mainCreatedLatch.await(1L, TimeUnit.SECONDS));
+ }
+ assertNull(client.checkExists().forPath(touchPath));
+ assertTrue(mainDeletedLatch.await(3L * testTtlMs,
TimeUnit.MILLISECONDS));
+ assertFalse(touchCreated.get()); // Just to control that touch
ZNode never created
+ }
+ }
+ }
}
