keith-turner commented on PR #5256:
URL: https://github.com/apache/accumulo/pull/5256#issuecomment-2613410890
> There were a lot of changes there, and I had to make some assumptions
based on removing exists calls. I also had to change ZcNode to allow data to be
null.
ZcNode should not allow data to be null because it only expects the data
constructor be called when there is data. When data is null means that data
was never fetched, which could be the case when only the children were fetched.
I have run into problems w/ this test before where its mocking of zookeeper
does not follow the behavior of zookeeper. When asking for data for a node
that does not exist in zookeeper it will throw a NoNodeException and not return
null. Made the following changes to add the non null check back and make the
test throw NoNodeException instead of return null. Also one test expected
synconnected to clear the cache, so had to change that. The syncconnected test
may be wrong in prev versions of Accumulo and maybe that is masked by the
incorrect mocking behavior.
```diff
diff --git
a/core/src/main/java/org/apache/accumulo/core/zookeeper/ZcNode.java
b/core/src/main/java/org/apache/accumulo/core/zookeeper/ZcNode.java
index 741f905b26..d1f75ae4b8 100644
--- a/core/src/main/java/org/apache/accumulo/core/zookeeper/ZcNode.java
+++ b/core/src/main/java/org/apache/accumulo/core/zookeeper/ZcNode.java
@@ -83,7 +83,7 @@ class ZcNode {
* stat.
*/
ZcNode(byte[] data, ZcStat zstat, ZcNode existing) {
- this.data = data;
+ this.data = Objects.requireNonNull(data);
this.stat = Objects.requireNonNull(zstat);
if (existing == null) {
this.children = null;
diff --git
a/core/src/test/java/org/apache/accumulo/core/zookeeper/ZooCacheTest.java
b/core/src/test/java/org/apache/accumulo/core/zookeeper/ZooCacheTest.java
index 02484b1692..6dd4e2dcc2 100644
--- a/core/src/test/java/org/apache/accumulo/core/zookeeper/ZooCacheTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/zookeeper/ZooCacheTest.java
@@ -263,7 +263,7 @@ public class ZooCacheTest {
@Test
public void testWatchDataNode_NoneSyncConnected() throws Exception {
- testWatchDataNode(null, Watcher.Event.EventType.None, false);
+ testWatchDataNode(null, Watcher.Event.EventType.None, true);
}
private void testWatchDataNode(byte[] initialData,
Watcher.Event.EventType eventType,
@@ -285,11 +285,11 @@ public class ZooCacheTest {
if (initialData != null) {
expect(zk.getData(ZPATH, null, existsStat)).andReturn(initialData);
} else {
- expect(zk.getData(ZPATH, null, existsStat)).andReturn(null);
+ expect(zk.getData(ZPATH, null, existsStat)).andThrow(new
KeeperException.NoNodeException(ZPATH));
}
replay(zk);
zc.get(ZPATH);
- assertEquals(initialData != null, zc.dataCached(ZPATH));
+ assertTrue(zc.dataCached(ZPATH));
}
@Test
```
I will look at the ephemeral thing and see if I know anything.
--
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
