You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "keith-turner (via GitHub)" <gi...@apache.org> on 2025/07/25 22:41:23 UTC

[PR] tests ZK restart and disconnect events for ZooCache [accumulo]

keith-turner opened a new pull request, #5765:
URL: https://github.com/apache/accumulo/pull/5765

   (no comment)


-- 
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@accumulo.apache.org

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


Re: [PR] tests ZK restart and disconnect events for ZooCache [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #5765:
URL: https://github.com/apache/accumulo/pull/5765


-- 
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@accumulo.apache.org

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


Re: [PR] tests ZK restart and disconnect events for ZooCache [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5765:
URL: https://github.com/apache/accumulo/pull/5765#discussion_r2232138156


##########
test/src/main/java/org/apache/accumulo/test/zookeeper/ZooCacheIT.java:
##########
@@ -228,4 +237,94 @@ public void testGetChildren() throws Exception {
           && zooCache.childrenCached(base + "/test3") == false;
     });
   }
+
+  @Test
+  public void testZookeeperRestart() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // restarting zookeeper should cause the cache to be cleared
+    szk.restart();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));

Review Comment:
   Its possible that persistent watchers were reregistered.  Would be good to check that after the even that cleared the cache that updates to ZK will still eventually be seen.



-- 
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@accumulo.apache.org

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


Re: [PR] tests ZK restart and disconnect events for ZooCache [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5765:
URL: https://github.com/apache/accumulo/pull/5765#discussion_r2237771408


##########
test/src/main/java/org/apache/accumulo/test/zookeeper/ZooCacheIT.java:
##########
@@ -228,4 +238,105 @@ public void testGetChildren() throws Exception {
           && zooCache.childrenCached(base + "/test3") == false;
     });
   }
+
+  @Test
+  public void testZookeeperRestart() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // restarting zookeeper should cause the cache to be cleared
+    szk.restart();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    // after the event, ensure that zoocache will still eventually see updates. May have
+    // reregistered watchers.
+    zrw.putPersistentData(base + "/test2", new byte[] {4, 3, 2, 1},
+        ZooUtil.NodeExistsPolicy.OVERWRITE);
+    Wait.waitFor(() -> Arrays.equals(new byte[] {4, 3, 2, 1}, zooCache.get(base + "/test2")));
+  }
+
+  @SuppressWarnings("deprecation")
+  @Test
+  public void testDisconnect() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // Find the zookeeper thread that sends stuff to the server and pause it for longer than the
+    // session timeout. This should cause the server to disconnect the session which should cause
+    // the cache to clear.
+    Thread sendThread = findZookeeperSendThread();
+    sendThread.suspend();
+    UtilWaitThread.sleep(SESSION_TIMEOUT.plusSeconds(1).toMillis());
+    sendThread.resume();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    // after the event, ensure that zoocache will still eventually see updates.
+    zrw.putPersistentData(base + "/test2", new byte[] {4, 3, 2, 1},
+        ZooUtil.NodeExistsPolicy.OVERWRITE);
+    Wait.waitFor(() -> Arrays.equals(new byte[] {4, 3, 2, 1}, zooCache.get(base + "/test2")));
+  }
+
+  private Thread findZookeeperSendThread() {
+    Map<Thread,StackTraceElement[]> traces = Thread.getAllStackTraces();
+    for (var entry : traces.entrySet()) {
+      Thread thread = entry.getKey();
+      StackTraceElement[] stackTrace = entry.getValue();
+      for (var ste : stackTrace) {
+        if (ste.getClassName().contains(ClientCnxn.class.getSimpleName() + "$SendThread")) {

Review Comment:
   Improved the error message in 55f80b01cc0f4d79fd1d4a3167048cc8f0dc5f73 when the class can not be found.  Would have gotten a NPE before that change.



-- 
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@accumulo.apache.org

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


Re: [PR] tests ZK restart and disconnect events for ZooCache [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5765:
URL: https://github.com/apache/accumulo/pull/5765#discussion_r2237403667


##########
test/src/main/java/org/apache/accumulo/test/zookeeper/ZooCacheIT.java:
##########
@@ -228,4 +238,105 @@ public void testGetChildren() throws Exception {
           && zooCache.childrenCached(base + "/test3") == false;
     });
   }
+
+  @Test
+  public void testZookeeperRestart() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // restarting zookeeper should cause the cache to be cleared
+    szk.restart();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    // after the event, ensure that zoocache will still eventually see updates. May have
+    // reregistered watchers.
+    zrw.putPersistentData(base + "/test2", new byte[] {4, 3, 2, 1},
+        ZooUtil.NodeExistsPolicy.OVERWRITE);
+    Wait.waitFor(() -> Arrays.equals(new byte[] {4, 3, 2, 1}, zooCache.get(base + "/test2")));
+  }
+
+  @SuppressWarnings("deprecation")
+  @Test
+  public void testDisconnect() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // Find the zookeeper thread that sends stuff to the server and pause it for longer than the
+    // session timeout. This should cause the server to disconnect the session which should cause
+    // the cache to clear.
+    Thread sendThread = findZookeeperSendThread();
+    sendThread.suspend();
+    UtilWaitThread.sleep(SESSION_TIMEOUT.plusSeconds(1).toMillis());
+    sendThread.resume();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    // after the event, ensure that zoocache will still eventually see updates.
+    zrw.putPersistentData(base + "/test2", new byte[] {4, 3, 2, 1},
+        ZooUtil.NodeExistsPolicy.OVERWRITE);
+    Wait.waitFor(() -> Arrays.equals(new byte[] {4, 3, 2, 1}, zooCache.get(base + "/test2")));
+  }
+
+  private Thread findZookeeperSendThread() {
+    Map<Thread,StackTraceElement[]> traces = Thread.getAllStackTraces();
+    for (var entry : traces.entrySet()) {
+      Thread thread = entry.getKey();
+      StackTraceElement[] stackTrace = entry.getValue();
+      for (var ste : stackTrace) {
+        if (ste.getClassName().contains(ClientCnxn.class.getSimpleName() + "$SendThread")) {

Review Comment:
   what do you mean by get it dynamically?  this code does find the thread at runtime.



-- 
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@accumulo.apache.org

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


Re: [PR] tests ZK restart and disconnect events for ZooCache [accumulo]

Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 commented on code in PR #5765:
URL: https://github.com/apache/accumulo/pull/5765#discussion_r2234091773


##########
test/src/main/java/org/apache/accumulo/test/zookeeper/ZooCacheIT.java:
##########
@@ -228,4 +238,105 @@ public void testGetChildren() throws Exception {
           && zooCache.childrenCached(base + "/test3") == false;
     });
   }
+
+  @Test
+  public void testZookeeperRestart() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));

Review Comment:
   ```suggestion
       final String root = Constants.ZROOT + UUID.randomUUID();
       final String base = root + Constants.ZTSERVERS;
       final String child = "test2";
       final String fullPath = base + "/" + child;
       final byte[] data1 = new byte[] {1, 2, 3, 4};
       final byte[] data2 = new byte[] {4, 3, 2, 1};
       TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
   ```
   Could add any of these



##########
test/src/main/java/org/apache/accumulo/test/zookeeper/ZooCacheIT.java:
##########
@@ -228,4 +238,105 @@ public void testGetChildren() throws Exception {
           && zooCache.childrenCached(base + "/test3") == false;
     });
   }
+
+  @Test
+  public void testZookeeperRestart() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // restarting zookeeper should cause the cache to be cleared
+    szk.restart();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    // after the event, ensure that zoocache will still eventually see updates. May have
+    // reregistered watchers.
+    zrw.putPersistentData(base + "/test2", new byte[] {4, 3, 2, 1},
+        ZooUtil.NodeExistsPolicy.OVERWRITE);
+    Wait.waitFor(() -> Arrays.equals(new byte[] {4, 3, 2, 1}, zooCache.get(base + "/test2")));
+  }
+
+  @SuppressWarnings("deprecation")
+  @Test
+  public void testDisconnect() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));

Review Comment:
   Could do same here, could use `test3` instead of `test2` here to distinguish the tests a bit more



##########
test/src/main/java/org/apache/accumulo/test/zookeeper/ZooCacheIT.java:
##########
@@ -228,4 +238,105 @@ public void testGetChildren() throws Exception {
           && zooCache.childrenCached(base + "/test3") == false;
     });
   }
+
+  @Test
+  public void testZookeeperRestart() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // restarting zookeeper should cause the cache to be cleared
+    szk.restart();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    // after the event, ensure that zoocache will still eventually see updates. May have
+    // reregistered watchers.
+    zrw.putPersistentData(base + "/test2", new byte[] {4, 3, 2, 1},
+        ZooUtil.NodeExistsPolicy.OVERWRITE);
+    Wait.waitFor(() -> Arrays.equals(new byte[] {4, 3, 2, 1}, zooCache.get(base + "/test2")));
+  }
+
+  @SuppressWarnings("deprecation")
+  @Test
+  public void testDisconnect() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // Find the zookeeper thread that sends stuff to the server and pause it for longer than the
+    // session timeout. This should cause the server to disconnect the session which should cause
+    // the cache to clear.
+    Thread sendThread = findZookeeperSendThread();
+    sendThread.suspend();
+    UtilWaitThread.sleep(SESSION_TIMEOUT.plusSeconds(1).toMillis());
+    sendThread.resume();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    // after the event, ensure that zoocache will still eventually see updates.
+    zrw.putPersistentData(base + "/test2", new byte[] {4, 3, 2, 1},
+        ZooUtil.NodeExistsPolicy.OVERWRITE);
+    Wait.waitFor(() -> Arrays.equals(new byte[] {4, 3, 2, 1}, zooCache.get(base + "/test2")));
+  }
+
+  private Thread findZookeeperSendThread() {
+    Map<Thread,StackTraceElement[]> traces = Thread.getAllStackTraces();
+    for (var entry : traces.entrySet()) {
+      Thread thread = entry.getKey();
+      StackTraceElement[] stackTrace = entry.getValue();
+      for (var ste : stackTrace) {
+        if (ste.getClassName().contains(ClientCnxn.class.getSimpleName() + "$SendThread")) {

Review Comment:
   Can we get SendThread here dynamically?



-- 
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@accumulo.apache.org

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


Re: [PR] tests ZK restart and disconnect events for ZooCache [accumulo]

Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 commented on code in PR #5765:
URL: https://github.com/apache/accumulo/pull/5765#discussion_r2237473005


##########
test/src/main/java/org/apache/accumulo/test/zookeeper/ZooCacheIT.java:
##########
@@ -228,4 +238,105 @@ public void testGetChildren() throws Exception {
           && zooCache.childrenCached(base + "/test3") == false;
     });
   }
+
+  @Test
+  public void testZookeeperRestart() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // restarting zookeeper should cause the cache to be cleared
+    szk.restart();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    // after the event, ensure that zoocache will still eventually see updates. May have
+    // reregistered watchers.
+    zrw.putPersistentData(base + "/test2", new byte[] {4, 3, 2, 1},
+        ZooUtil.NodeExistsPolicy.OVERWRITE);
+    Wait.waitFor(() -> Arrays.equals(new byte[] {4, 3, 2, 1}, zooCache.get(base + "/test2")));
+  }
+
+  @SuppressWarnings("deprecation")
+  @Test
+  public void testDisconnect() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // Find the zookeeper thread that sends stuff to the server and pause it for longer than the
+    // session timeout. This should cause the server to disconnect the session which should cause
+    // the cache to clear.
+    Thread sendThread = findZookeeperSendThread();
+    sendThread.suspend();
+    UtilWaitThread.sleep(SESSION_TIMEOUT.plusSeconds(1).toMillis());
+    sendThread.resume();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    // after the event, ensure that zoocache will still eventually see updates.
+    zrw.putPersistentData(base + "/test2", new byte[] {4, 3, 2, 1},
+        ZooUtil.NodeExistsPolicy.OVERWRITE);
+    Wait.waitFor(() -> Arrays.equals(new byte[] {4, 3, 2, 1}, zooCache.get(base + "/test2")));
+  }
+
+  private Thread findZookeeperSendThread() {
+    Map<Thread,StackTraceElement[]> traces = Thread.getAllStackTraces();
+    for (var entry : traces.entrySet()) {
+      Thread thread = entry.getKey();
+      StackTraceElement[] stackTrace = entry.getValue();
+      for (var ste : stackTrace) {
+        if (ste.getClassName().contains(ClientCnxn.class.getSimpleName() + "$SendThread")) {

Review Comment:
   I meant the inner SendThread class string "$SendThread" in case that class name changes in the future. Might not be possible



-- 
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@accumulo.apache.org

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


Re: [PR] tests ZK restart and disconnect events for ZooCache [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5765:
URL: https://github.com/apache/accumulo/pull/5765#discussion_r2237399109


##########
test/src/main/java/org/apache/accumulo/test/zookeeper/ZooCacheIT.java:
##########
@@ -228,4 +238,105 @@ public void testGetChildren() throws Exception {
           && zooCache.childrenCached(base + "/test3") == false;
     });
   }
+
+  @Test
+  public void testZookeeperRestart() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // restarting zookeeper should cause the cache to be cleared
+    szk.restart();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    // after the event, ensure that zoocache will still eventually see updates. May have
+    // reregistered watchers.
+    zrw.putPersistentData(base + "/test2", new byte[] {4, 3, 2, 1},
+        ZooUtil.NodeExistsPolicy.OVERWRITE);
+    Wait.waitFor(() -> Arrays.equals(new byte[] {4, 3, 2, 1}, zooCache.get(base + "/test2")));
+  }
+
+  @SuppressWarnings("deprecation")
+  @Test
+  public void testDisconnect() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));

Review Comment:
   Made some changes to consolidate duplicate code rather then copy the changes you made in the other method.  



##########
test/src/main/java/org/apache/accumulo/test/zookeeper/ZooCacheIT.java:
##########
@@ -228,4 +238,105 @@ public void testGetChildren() throws Exception {
           && zooCache.childrenCached(base + "/test3") == false;
     });
   }
+
+  @Test
+  public void testZookeeperRestart() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));
+
+    zrw.mkdirs(base);
+    zrw.putPersistentData(base + "/test2", new byte[] {1, 2, 3, 4}, ZooUtil.NodeExistsPolicy.FAIL);
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    long uc1 = zooCache.getUpdateCount();
+
+    assertTrue(zooCache.dataCached(base + "/test2"));
+    assertTrue(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    assertEquals(uc1, zooCache.getUpdateCount());
+
+    // restarting zookeeper should cause the cache to be cleared
+    szk.restart();
+
+    // clearing the cache should increment the update count
+    Wait.waitFor(() -> uc1 != zooCache.getUpdateCount());
+    // The data and children previously cached should no longer be cached
+    assertFalse(zooCache.dataCached(base + "/test2"));
+    assertFalse(zooCache.childrenCached(base));
+
+    assertArrayEquals(new byte[] {1, 2, 3, 4}, zooCache.get(base + "/test2"));
+    assertEquals(List.of("test2"), zooCache.getChildren(base));
+
+    // after the event, ensure that zoocache will still eventually see updates. May have
+    // reregistered watchers.
+    zrw.putPersistentData(base + "/test2", new byte[] {4, 3, 2, 1},
+        ZooUtil.NodeExistsPolicy.OVERWRITE);
+    Wait.waitFor(() -> Arrays.equals(new byte[] {4, 3, 2, 1}, zooCache.get(base + "/test2")));
+  }
+
+  @SuppressWarnings("deprecation")
+  @Test
+  public void testDisconnect() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));

Review Comment:
   Made some changes to consolidate duplicate code rather than copy the changes you made in the other method.  



-- 
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@accumulo.apache.org

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


Re: [PR] tests ZK restart and disconnect events for ZooCache [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5765:
URL: https://github.com/apache/accumulo/pull/5765#discussion_r2237425757


##########
test/src/main/java/org/apache/accumulo/test/zookeeper/ZooCacheIT.java:
##########
@@ -228,4 +238,105 @@ public void testGetChildren() throws Exception {
           && zooCache.childrenCached(base + "/test3") == false;
     });
   }
+
+  @Test
+  public void testZookeeperRestart() throws Exception {
+    final String root = Constants.ZROOT + UUID.randomUUID();
+    final String base = root + Constants.ZTSERVERS;
+    TestZooCache zooCache = new TestZooCache(zk, Set.of(base));

Review Comment:
   changed data1 and data2 to strings in a later commit, wanted to detect if the code under test modified the bye array



-- 
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@accumulo.apache.org

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