kezhuw commented on code in PR #1260:
URL: https://github.com/apache/curator/pull/1260#discussion_r2022798278
##########
curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentTtlNode.java:
##########
@@ -187,4 +187,34 @@ public void childEvent(CuratorFramework client,
PathChildrenCacheEvent event) th
assertNull(client.checkExists().forPath("/test"));
}
}
+
+ @Test
+ public void testTouchNodeNotCreated() throws Exception {
+ try (CuratorFramework client =
+ CuratorFrameworkFactory.newClient(server.getConnectString(),
new RetryOneTime(1))) {
+ client.start();
+ final long ttlMs = 1_000L;
+ try (PersistentTtlNode node = new PersistentTtlNode(client,
"/test", ttlMs, new byte[0])) {
+ node.start();
+ assertTrue(node.waitForInitialCreate(timing.session(),
TimeUnit.MILLISECONDS));
+ // Give some minor time for touch node to be created. Will
worked after patch
Review Comment:
I think we are testing a situation that "touch" node is not scheduled to be
created. May be we can create a overrideable method, say "scheduleTouchTask",
to use for test.
##########
curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentTtlNode.java:
##########
@@ -187,4 +187,34 @@ public void childEvent(CuratorFramework client,
PathChildrenCacheEvent event) th
assertNull(client.checkExists().forPath("/test"));
}
}
+
+ @Test
+ public void testTouchNodeNotCreated() throws Exception {
+ try (CuratorFramework client =
+ CuratorFrameworkFactory.newClient(server.getConnectString(),
new RetryOneTime(1))) {
+ client.start();
+ final long ttlMs = 1_000L;
+ try (PersistentTtlNode node = new PersistentTtlNode(client,
"/test", ttlMs, new byte[0])) {
+ node.start();
+ assertTrue(node.waitForInitialCreate(timing.session(),
TimeUnit.MILLISECONDS));
+ // Give some minor time for touch node to be created. Will
worked after patch
+ for (int i = 1; i <= 5; i++) {
+ if (client.checkExists().forPath("/test") != null) {
+ break;
+ }
+ Thread.sleep(10L);
+ }
+ }
+ }
+ try (CuratorFramework client1 =
+ CuratorFrameworkFactory.newClient(server.getConnectString(),
new RetryOneTime(1))) {
+ client1.start();
+ assertTrue(client1.blockUntilConnected(2, TimeUnit.SECONDS));
+ Thread.sleep(3_000L);
Review Comment:
```suggestion
Thread.sleep(N * ttlMs);
```
##########
curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentTtlNode.java:
##########
@@ -187,4 +187,34 @@ public void childEvent(CuratorFramework client,
PathChildrenCacheEvent event) th
assertNull(client.checkExists().forPath("/test"));
}
}
+
+ @Test
+ public void testTouchNodeNotCreated() throws Exception {
+ try (CuratorFramework client =
+ CuratorFrameworkFactory.newClient(server.getConnectString(),
new RetryOneTime(1))) {
+ client.start();
+ final long ttlMs = 1_000L;
+ try (PersistentTtlNode node = new PersistentTtlNode(client,
"/test", ttlMs, new byte[0])) {
+ node.start();
+ assertTrue(node.waitForInitialCreate(timing.session(),
TimeUnit.MILLISECONDS));
+ // Give some minor time for touch node to be created. Will
worked after patch
+ for (int i = 1; i <= 5; i++) {
+ if (client.checkExists().forPath("/test") != null) {
+ break;
+ }
+ Thread.sleep(10L);
+ }
+ }
+ }
+ try (CuratorFramework client1 =
+ CuratorFrameworkFactory.newClient(server.getConnectString(),
new RetryOneTime(1))) {
+ client1.start();
+ assertTrue(client1.blockUntilConnected(2, TimeUnit.SECONDS));
+ Thread.sleep(3_000L);
+ assertNull(client1.checkExists().forPath("/test/touch"));
Review Comment:
We could move this before `sleep` to ensure that the "touch" node is not
created.
##########
curator-recipes/src/main/java/org/apache/curator/framework/recipes/nodes/PersistentTtlNode.java:
##########
@@ -239,8 +241,8 @@ public byte[] getData() {
/**
* Call when you are done with the PersistentTtlNode. Note: the ZNode is
<em>not</em> immediately
- * deleted. However, if no other PersistentTtlNode with the same path is
running the node will get deleted
- * based on the ttl.
+ * deleted. However, if no other PersistentTtlNode with the same path is
running the node will get
+ * deleted based on the ttl.
Review Comment:
It is good to keep comments untouch if there is no meaningful content
changes.
##########
curator-recipes/src/test/java/org/apache/curator/framework/recipes/nodes/TestPersistentTtlNode.java:
##########
@@ -187,4 +187,34 @@ public void childEvent(CuratorFramework client,
PathChildrenCacheEvent event) th
assertNull(client.checkExists().forPath("/test"));
}
}
+
+ @Test
+ public void testTouchNodeNotCreated() throws Exception {
+ try (CuratorFramework client =
+ CuratorFrameworkFactory.newClient(server.getConnectString(),
new RetryOneTime(1))) {
+ client.start();
+ final long ttlMs = 1_000L;
+ try (PersistentTtlNode node = new PersistentTtlNode(client,
"/test", ttlMs, new byte[0])) {
+ node.start();
+ assertTrue(node.waitForInitialCreate(timing.session(),
TimeUnit.MILLISECONDS));
+ // Give some minor time for touch node to be created. Will
worked after patch
+ for (int i = 1; i <= 5; i++) {
+ if (client.checkExists().forPath("/test") != null) {
+ break;
+ }
+ Thread.sleep(10L);
+ }
+ }
+ }
+ try (CuratorFramework client1 =
Review Comment:
I don't think we need yet another client.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
