keith-turner commented on code in PR #5256:
URL: https://github.com/apache/accumulo/pull/5256#discussion_r1930880210
##########
core/src/main/java/org/apache/accumulo/core/zookeeper/ZcNode.java:
##########
@@ -98,7 +98,6 @@ private ZcNode() {
* @throws IllegalStateException in the case where the node exists and the
data was never set
*/
byte[] getData() {
- Preconditions.checkState(cachedData());
Review Comment:
Hopefully this Preconditions can be added back now. That check is to
defend against calling getData when data was never fetched from zookeeper, in
which case a null would returned indicating zoocache knows the node does not
exist when that is not true. Guessing the removal was related to the incorrect
mocking of ZK in the unit test.
##########
test/src/main/java/org/apache/accumulo/test/functional/ZooCacheIT.java:
##########
@@ -81,4 +100,149 @@ public void test() throws Exception {
}
}
+ @Test
+ public void testZooKeeperClientChanges() throws Exception {
+
+ try (final AccumuloClient client =
Accumulo.newClient().from(getClientProperties()).build();
+ final ClientContext ctx = ((ClientContext) client)) {
+
+ final InstanceId iid = ctx.getInstanceID();
+ final String zooRoot = ZooUtil.getRoot(iid);
+ final String tableName = "test_Table";
+
+ final AtomicBoolean tableCreatedEvent = new AtomicBoolean(false);
+ final AtomicBoolean tableDeletedEvent = new AtomicBoolean(false);
+ final AtomicBoolean connectionOpenEvent = new AtomicBoolean(false);
+ final AtomicBoolean connectionClosedEvent = new AtomicBoolean(false);
+ final AtomicBoolean sessionExpiredEvent = new AtomicBoolean(false);
+ final AtomicReference<String> tableId = new AtomicReference<>();
+ final Supplier<AtomicReference<String>> tableIdSup = () -> tableId;
+
+ final ZooCache cache = ctx.getZooCache();
+ cache.addZooCacheWatcher(new ZooCacheWatcher() {
+
+ @Override
+ @SuppressWarnings("deprecation")
+ public void accept(WatchedEvent event) {
+
+ final String eventPath = event.getPath();
+
+ if (event.getType() != EventType.None
+ && !eventPath.startsWith(zooRoot + Constants.ZTABLES)) {
+ return;
+ }
+ // LOG.info("received event {}", event);
+
+ String tableId = tableIdSup.get().get();
+
+ switch (event.getType()) {
+ case ChildWatchRemoved:
+ case DataWatchRemoved:
+ case NodeChildrenChanged:
+ case NodeDataChanged:
+ case PersistentWatchRemoved:
+ break;
+ case NodeCreated:
+ if (eventPath.equals(zooRoot + Constants.ZTABLES + "/" +
tableId)) {
+ LOG.info("Setting tableCreatedEvent");
+ tableCreatedEvent.set(true);
+ }
+ break;
+ case NodeDeleted:
+ if (eventPath.equals(zooRoot + Constants.ZTABLES + "/" +
tableId)) {
+ LOG.info("Setting tableDeletedEvent");
+ tableDeletedEvent.set(true);
+ }
+ break;
+ case None:
+ switch (event.getState()) {
+ case AuthFailed:
+ case ConnectedReadOnly:
+ case NoSyncConnected:
+ case SaslAuthenticated:
+ case Unknown:
+ break;
+ case Closed:
+ case Disconnected:
+ LOG.info("Setting connectionClosedEvent");
+ connectionClosedEvent.set(true);
+ break;
+ case Expired:
+ LOG.info("Setting sessionExpiredEvent");
+ sessionExpiredEvent.set(true);
+ break;
+ case SyncConnected:
+ LOG.info("Setting connectionOpenEvent");
+ connectionOpenEvent.set(true);
+ break;
+ default:
+ break;
+
+ }
+ break;
+ default:
+ break;
+
+ }
+ }
+
+ });
+
+ assertFalse(connectionOpenEvent.get()); // missed the event, connection
already established
+
+ client.tableOperations().create(tableName);
+ final String tid = ctx.tableOperations().tableIdMap().get(tableName);
+ tableId.set(tid);
+ // we might miss the table created event, don't check for it
+ final String tableZPath = zooRoot + Constants.ZTABLES + "/" + tid;
+ assertFalse(cache.childrenCached(tableZPath));
+ assertNotNull(cache.getChildren(tableZPath));
+ assertTrue(cache.childrenCached(tableZPath));
+
+ client.tableOperations().delete(tableName);
+ Wait.waitFor(() -> tableDeletedEvent.get(), 60_000);
+ assertFalse(cache.childrenCached(tableZPath)); // ZooCache watcher fired
and deleted the data
+ assertNull(cache.getChildren(tableZPath));
+
+ assertFalse(connectionOpenEvent.get());
+ assertFalse(connectionClosedEvent.get());
+ tableDeletedEvent.set(false);
+
+ getCluster().getClusterControl().stop(ServerType.ZOOKEEPER);
Review Comment:
I like this test. In a follow on we could explore pausing the ZK process
using SIGSTOP and SIGCONT in addition to killing it. Like pause the ZK process
for 1.5x the configured ZK timeout and see if things behave as expected.
Not sure if this would change anything for this test, wondering if
configuring the zookeeper timeout to lower than the default of 30s would help
w/ runtimes for this test.
--
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-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
