This is an automated email from the ASF dual-hosted git repository.
dlmarion pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push:
new 5c8c202723 Reverted setting interrupted flag in ZooCache (#6552)
5c8c202723 is described below
commit 5c8c20272357952da84149fcfeb70cb329f26466
Author: Dave Marion <[email protected]>
AuthorDate: Wed Sep 23 06:19:26 2026 -0400
Reverted setting interrupted flag in ZooCache (#6552)
PR #6438 set the interrupted flag when InteruptedException
is caught, and subsequently some of those modifications have
been reverted. This is another one of those cases as in some
instances where the application is interrupted this ends up
in an endless loop.
This is a behavior change for ZooCache as before this change
ZooCache would swallow the InterruptedException and keep
retrying. Now it will throw an IllegalStateException.
---
.../accumulo/core/fate/zookeeper/ZooCache.java | 54 ++++++++++------------
.../apache/accumulo/core/util/UtilWaitThread.java | 1 -
.../accumulo/core/fate/zookeeper/ZooCacheTest.java | 28 +++++++----
3 files changed, 45 insertions(+), 38 deletions(-)
diff --git
a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooCache.java
b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooCache.java
index 714bbb17c7..5847401c72 100644
--- a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooCache.java
+++ b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooCache.java
@@ -247,39 +247,35 @@ public class ZooCache {
int sleepTime = 100;
- while (true) {
-
- try {
- return run();
- } catch (KeeperException e) {
- final Code code = e.code();
- if (code == Code.NONODE) {
- log.error("Looked up non-existent node in cache " + e.getPath(),
e);
- } else if (code == Code.CONNECTIONLOSS || code ==
Code.OPERATIONTIMEOUT
- || code == Code.SESSIONEXPIRED) {
- log.warn("Saw (possibly) transient exception communicating with
ZooKeeper, will retry",
- e);
- } else {
- log.warn("Zookeeper error, will retry", e);
+ try {
+ while (true) {
+
+ try {
+ return run();
+ } catch (KeeperException e) {
+ final Code code = e.code();
+ if (code == Code.NONODE) {
+ log.error("Looked up non-existent node in cache " + e.getPath(),
e);
+ } else if (code == Code.CONNECTIONLOSS || code ==
Code.OPERATIONTIMEOUT
+ || code == Code.SESSIONEXPIRED) {
+ log.warn(
+ "Saw (possibly) transient exception communicating with
ZooKeeper, will retry", e);
+ } else {
+ log.warn("Zookeeper error, will retry", e);
+ }
+ } catch (ConcurrentModificationException e) {
+ log.debug("Zookeeper was modified, will retry");
}
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- log.info("Zookeeper error, will retry", e);
- } catch (ConcurrentModificationException e) {
- log.debug("Zookeeper was modified, will retry");
- }
- try {
- // do not hold lock while sleeping
Thread.sleep(sleepTime);
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- log.debug("Wait in retry() was interrupted.", e);
- }
- LockSupport.parkNanos(sleepTime);
- if (sleepTime < 10_000) {
- sleepTime = (int) (sleepTime + sleepTime * random.nextDouble());
+ LockSupport.parkNanos(sleepTime);
+ if (sleepTime < 10_000) {
+ sleepTime = (int) (sleepTime + sleepTime * random.nextDouble());
+ }
}
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException("ZooCache thread interrupted.", e);
}
}
diff --git
a/core/src/main/java/org/apache/accumulo/core/util/UtilWaitThread.java
b/core/src/main/java/org/apache/accumulo/core/util/UtilWaitThread.java
index 08af3a867c..5e440be9b6 100644
--- a/core/src/main/java/org/apache/accumulo/core/util/UtilWaitThread.java
+++ b/core/src/main/java/org/apache/accumulo/core/util/UtilWaitThread.java
@@ -58,7 +58,6 @@ public class UtilWaitThread {
NANOSECONDS.sleep(remainingNanos);
return;
} catch (InterruptedException e) {
- Thread.currentThread().interrupt();
interrupted = true;
remainingNanos = end - System.nanoTime();
}
diff --git
a/core/src/test/java/org/apache/accumulo/core/fate/zookeeper/ZooCacheTest.java
b/core/src/test/java/org/apache/accumulo/core/fate/zookeeper/ZooCacheTest.java
index 39a2184568..a59fbfbfb1 100644
---
a/core/src/test/java/org/apache/accumulo/core/fate/zookeeper/ZooCacheTest.java
+++
b/core/src/test/java/org/apache/accumulo/core/fate/zookeeper/ZooCacheTest.java
@@ -32,6 +32,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.List;
@@ -132,11 +133,16 @@ public class ZooCacheTest {
private void testGet_Retry(Exception e) throws Exception {
expect(zk.exists(eq(ZPATH), anyObject(Watcher.class))).andThrow(e);
Stat existsStat = new Stat();
- expect(zk.exists(eq(ZPATH),
anyObject(Watcher.class))).andReturn(existsStat);
- expect(zk.getData(eq(ZPATH), anyObject(Watcher.class),
eq(existsStat))).andReturn(DATA);
- replay(zk);
- assertArrayEquals(DATA, zc.get(ZPATH));
+ if (e instanceof InterruptedException) {
+ replay(zk);
+ assertThrows(IllegalStateException.class, () -> zc.get(ZPATH));
+ } else {
+ expect(zk.exists(eq(ZPATH),
anyObject(Watcher.class))).andReturn(existsStat);
+ expect(zk.getData(eq(ZPATH), anyObject(Watcher.class),
eq(existsStat))).andReturn(DATA);
+ replay(zk);
+ assertArrayEquals(DATA, zc.get(ZPATH));
+ }
verify(zk);
}
@@ -160,15 +166,21 @@ public class ZooCacheTest {
testGet_Retry2(new InterruptedException());
}
+ // Like the other method, but throws on getData instead of exists
private void testGet_Retry2(Exception e) throws Exception {
Stat existsStat = new Stat();
expect(zk.exists(eq(ZPATH),
anyObject(Watcher.class))).andReturn(existsStat);
expect(zk.getData(eq(ZPATH), anyObject(Watcher.class),
eq(existsStat))).andThrow(e);
- expect(zk.exists(eq(ZPATH),
anyObject(Watcher.class))).andReturn(existsStat);
- expect(zk.getData(eq(ZPATH), anyObject(Watcher.class),
eq(existsStat))).andReturn(DATA);
- replay(zk);
- assertArrayEquals(DATA, zc.get(ZPATH));
+ if (e instanceof InterruptedException) {
+ replay(zk);
+ assertThrows(IllegalStateException.class, () -> zc.get(ZPATH));
+ } else {
+ expect(zk.exists(eq(ZPATH),
anyObject(Watcher.class))).andReturn(existsStat);
+ expect(zk.getData(eq(ZPATH), anyObject(Watcher.class),
eq(existsStat))).andReturn(DATA);
+ replay(zk);
+ assertArrayEquals(DATA, zc.get(ZPATH));
+ }
verify(zk);
}
