kevinrr888 commented on code in PR #5286:
URL: https://github.com/apache/accumulo/pull/5286#discussion_r1941910798
##########
core/src/main/java/org/apache/accumulo/core/zookeeper/ZcNode.java:
##########
@@ -29,27 +29,42 @@
* and children are obtained from zookeeper at different times. This class is
structured so that
* data can be obtained first and then children added later or visa veras.
*
- * <p>
- * Four distinct states can be cached for a zookeeper node.
- * <ul>
- * <li>Can cache that a node does not exist in zookeeper. This state is
represented by data, state,
- * and children all being null.</li>
- * <li>Can cache only the data for a zookeeper node. For this state data and
stat are non-null while
- * children is null. Calling getChildren on node in this state will throw an
exception.</li>
- * <li>Can cache only the children for a zookeeper node. For this state
children is non-null while
- * data and stat are null. Calling getData or getStat on node in this state
will throw an
- * exception.</li>
- * <li>Can cache the children and data for a zookeeper node. For this state
data,stat, and children
- * are non-null.</li>
- * </ul>
- * <p>
- *
*/
class ZcNode {
+ /**
+ * This enum represents what ZooCache has discovered about a given node in
zookeeper so far.
+ */
+ enum Discovered {
+ /**
+ * An attempt was made to fetch data or children and ZooKeeper threw a
NoNodeException. In this
+ * case ZooCache knows the node did not exist.
+ */
+ NON_EXISTENT,
+ /**
+ * ZooCache knows the node existed and what its children were because a
successful call was made
+ * to ZooKepper to get children. However zoocache has never requested the
nodes data and does
Review Comment:
```suggestion
* to ZooKeeper to get children. However zoocache has never requested
the nodes data and does
```
##########
core/src/main/java/org/apache/accumulo/core/zookeeper/ZcNode.java:
##########
@@ -29,27 +29,42 @@
* and children are obtained from zookeeper at different times. This class is
structured so that
* data can be obtained first and then children added later or visa veras.
*
- * <p>
- * Four distinct states can be cached for a zookeeper node.
- * <ul>
- * <li>Can cache that a node does not exist in zookeeper. This state is
represented by data, state,
- * and children all being null.</li>
- * <li>Can cache only the data for a zookeeper node. For this state data and
stat are non-null while
- * children is null. Calling getChildren on node in this state will throw an
exception.</li>
- * <li>Can cache only the children for a zookeeper node. For this state
children is non-null while
- * data and stat are null. Calling getData or getStat on node in this state
will throw an
- * exception.</li>
Review Comment:
Could keep the comments regarding what method calls in what state will
result in an exception
##########
core/src/main/java/org/apache/accumulo/core/zookeeper/ZcNode.java:
##########
@@ -29,27 +29,42 @@
* and children are obtained from zookeeper at different times. This class is
structured so that
* data can be obtained first and then children added later or visa veras.
*
- * <p>
- * Four distinct states can be cached for a zookeeper node.
- * <ul>
- * <li>Can cache that a node does not exist in zookeeper. This state is
represented by data, state,
- * and children all being null.</li>
- * <li>Can cache only the data for a zookeeper node. For this state data and
stat are non-null while
- * children is null. Calling getChildren on node in this state will throw an
exception.</li>
- * <li>Can cache only the children for a zookeeper node. For this state
children is non-null while
- * data and stat are null. Calling getData or getStat on node in this state
will throw an
- * exception.</li>
- * <li>Can cache the children and data for a zookeeper node. For this state
data,stat, and children
- * are non-null.</li>
- * </ul>
- * <p>
- *
*/
class ZcNode {
+ /**
+ * This enum represents what ZooCache has discovered about a given node in
zookeeper so far.
+ */
+ enum Discovered {
+ /**
+ * An attempt was made to fetch data or children and ZooKeeper threw a
NoNodeException. In this
+ * case ZooCache knows the node did not exist.
+ */
+ NON_EXISTENT,
+ /**
+ * ZooCache knows the node existed and what its children were because a
successful call was made
+ * to ZooKepper to get children. However zoocache has never requested the
nodes data and does
+ * not know its data.
+ */
+ CHILDREN_ONLY,
+ /**
+ * ZooCache knows the node exist and what its data was because a
successful call was mde to
Review Comment:
```suggestion
* ZooCache knows the node exists and what its data was because a
successful call was made to
```
##########
core/src/main/java/org/apache/accumulo/core/zookeeper/ZcNode.java:
##########
@@ -83,13 +116,29 @@ private ZcNode() {
* stat.
*/
ZcNode(byte[] data, ZcStat zstat, ZcNode existing) {
- this.data = Objects.requireNonNull(data);
- this.stat = Objects.requireNonNull(zstat);
Review Comment:
Does not make a difference so feel free to ignore: I think it would be a bit
clearer to keep these checks at the top of the constructor instead of at the
end.
--
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]
