Apache9 commented on code in PR #5954:
URL: https://github.com/apache/hbase/pull/5954#discussion_r1634917093
##########
hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java:
##########
@@ -168,6 +168,28 @@ public static int checkExists(ZKWatcher zkw, String znode)
throws KeeperExceptio
}
}
+ /**
+ * Get the create time if the specified node exists.
+ * @param zkw zk reference
+ * @param znode path of node to watch
+ * @return create time of the node if it exists, -1 if does not exist
+ * @throws KeeperException if unexpected zookeeper exception
+ */
+ public static long getCreateTimeIfNodeExists(ZKWatcher zkw, String znode)
throws KeeperException {
+ try {
+ Stat s = zkw.getRecoverableZooKeeper().exists(znode, false);
+ return s != null ? s.getCtime() : -1;
+ } catch (KeeperException e) {
+ LOG.warn(zkw.prefix("Unable to get create time on znode (" + znode +
")"), e);
+ zkw.keeperException(e);
+ return -1;
Review Comment:
Should we return -1 here instead of throwing exception out?
##########
hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ZKReplicationPeerStorage.java:
##########
@@ -185,6 +185,17 @@ public ReplicationPeerConfig getPeerConfig(String peerId)
throws ReplicationExce
}
}
+ @Override
+ public long getPeerCreateTime(String peerId) throws ReplicationException {
+ long createTimeIfNodeExists;
+ try {
+ createTimeIfNodeExists = ZKUtil.getCreateTimeIfNodeExists(zookeeper,
getPeerNode(peerId));
Review Comment:
Why not just return here? And what is the return value if the znode does not
exist?
##########
hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java:
##########
@@ -168,6 +168,28 @@ public static int checkExists(ZKWatcher zkw, String znode)
throws KeeperExceptio
}
}
+ /**
+ * Get the create time if the specified node exists.
+ * @param zkw zk reference
+ * @param znode path of node to watch
+ * @return create time of the node if it exists, -1 if does not exist
+ * @throws KeeperException if unexpected zookeeper exception
+ */
+ public static long getCreateTimeIfNodeExists(ZKWatcher zkw, String znode)
throws KeeperException {
+ try {
+ Stat s = zkw.getRecoverableZooKeeper().exists(znode, false);
+ return s != null ? s.getCtime() : -1;
Review Comment:
OK, -1, then we'd better have a constant in the ReplicationPeerStorage
interface so all the implementations follow the same pattern?
##########
hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/FSReplicationPeerStorage.java:
##########
@@ -227,6 +227,16 @@ public ReplicationPeerConfig getPeerConfig(String peerId)
throws ReplicationExce
}
}
+ @Override
+ public long getPeerCreateTime(String peerId) throws ReplicationException {
+ Path peerDir = getPeerDir(peerId);
+ try {
+ return fs.getFileStatus(peerDir).getModificationTime();
+ } catch (IOException e) {
Review Comment:
We need to check null or catch FileNotFoundException here in case the peer
does not exist?
--
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]
