pkuwm commented on a change in pull request #1035:
URL: https://github.com/apache/helix/pull/1035#discussion_r432237007
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -1270,6 +1325,9 @@ public void deleteRecursively(String path) throws
ZkClientException {
private void processDataOrChildChange(WatchedEvent event, long
notificationTime) {
final String path = event.getPath();
final boolean pathExists = event.getType() != EventType.NodeDeleted;
+ if (EventType.NodeDeleted == event.getType()) {
+ LOG.debug("event delelete:" + event.getPath());
Review comment:
@kaisun2000 Typo. And could you change the logging to parameterized
logging: `LOG.debug("Event delete: {}", event.getPath());`? There is
performance advantage over the concatenation. The fully formatted message text
is NOT created unless the DEBUG level is enabled for the logger. It is like
```
if (logger.isDebugEnabled()) {
LOG.debug("event delete: " + event.getPath());
}
```
But the one you use will still create the message. We would like optimal
performance, right?
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -1054,15 +1078,46 @@ public Stat getStat(final String path) {
}
private Stat getStat(final String path, final boolean watch) {
+ return getStat(path, watch, false);
+ }
+
+ /*
+ * Install watch if there is such node and return the stat
+ *
+ * If useGetData false, use exist(). @param watch would determine if adding
watch
+ * to ZooKeeper server or not.
+ * Note, if @param path does not exist in ZooKeeper server, watch would
still be installed
+ * if @param watch is true.
+ *
+ * If useGetData true, use getData() to add watch. ignore @param watch in
this case.
+ * Note, if @param path does not exist in ZooKeeper server, no watch would
be added.
+ */
+ private Stat getStat(final String path, final boolean watch, final boolean
useGetData) {
long startT = System.currentTimeMillis();
+ Stat stat = null;
try {
- Stat stat = retryUntilConnected(
- () -> ((ZkConnection) getConnection()).getZookeeper().exists(path,
watch));
+ if (!useGetData) {
+ stat = retryUntilConnected(
+ () -> ((ZkConnection) getConnection()).getZookeeper().exists(path,
watch));
+ } else {
+ stat = new Stat();
+ Stat finalStat = stat;
Review comment:
Is this variable unnecessary?
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/api/client/ChildrenSubscribeResult.java
##########
@@ -0,0 +1,27 @@
+package org.apache.helix.zookeeper.api.client;
+
Review comment:
Apache license.
##########
File path:
zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
##########
@@ -1054,15 +1082,47 @@ public Stat getStat(final String path) {
}
private Stat getStat(final String path, final boolean watch) {
+ return getStat(path, watch, false);
+ }
+
+ /*
+ * Install watch if there is such node and return the stat
+ *
+ * If useGetData false, use exist(). @param watch would determine if adding
watch
+ * to ZooKeeper server or not.
+ * Note, if @param path does not exist in ZooKeeper server, watch would
still be installed
+ * if @param watch is true.
+ *
+ * If useGetData true, use getData() to add watch. ignore @param watch in
this case.
+ * Note, if @param path does not exist in ZooKeeper server, no watch would
be added.
+ */
+ private Stat getStat(final String path, final boolean watch, final boolean
useGetData) {
long startT = System.currentTimeMillis();
+ Stat stat = null;
try {
- Stat stat = retryUntilConnected(
- () -> ((ZkConnection) getConnection()).getZookeeper().exists(path,
watch));
+ if (!useGetData) {
+ stat = retryUntilConnected(
+ () -> ((ZkConnection) getConnection()).getZookeeper().exists(path,
watch));
+ } else {
+ stat = new Stat();
+ try {
+ LOG.debug("getstat() invoked with useGetData() with path: " + path);
+ Stat finalStat = stat;
Review comment:
Why is this `finaStat` necessary?
##########
File path:
helix-core/src/test/java/org/apache/helix/integration/manager/ClusterManager.java
##########
@@ -0,0 +1,83 @@
+package org.apache.helix.integration.manager;
+
Review comment:
Apache license.
##########
File path:
helix-core/src/test/java/org/apache/helix/integration/manager/ClusterSpectatorManager.java
##########
@@ -0,0 +1,30 @@
+package org.apache.helix.integration.manager;
+
Review comment:
Apache license.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
