HBASE-28631 Show the create time of the replication peer #5954
+89
−7
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Apache9
reviewed
Jun 3, 2024
|
Ok, I understand |
Could we just use the creation time of the peer id znode or directory? If this is not stable, we could introduce a separated znode or file to store it. |
The latest code has been submitted, please review it if you have time. I use the creation time of the peer id znode to get the create time of the peer. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Apache9
reviewed
Jun 11, 2024
...-replication/src/main/java/org/apache/hadoop/hbase/replication/FSReplicationPeerStorage.java
Show resolved
Hide resolved
| public long getPeerCreateTime(String peerId) throws ReplicationException { | ||
| long createTimeIfNodeExists; | ||
| try { | ||
| createTimeIfNodeExists = ZKUtil.getCreateTimeIfNodeExists(zookeeper, getPeerNode(peerId)); |
There was a problem hiding this comment.
Why not just return here? And what is the return value if the znode does not exist?
| public static long getCreateTimeIfNodeExists(ZKWatcher zkw, String znode) throws KeeperException { | ||
| try { | ||
| Stat s = zkw.getRecoverableZooKeeper().exists(znode, false); | ||
| return s != null ? s.getCtime() : -1; |
There was a problem hiding this comment.
OK, -1, then we'd better have a constant in the ReplicationPeerStorage interface so all the implementations follow the same pattern?
| } catch (KeeperException e) { | ||
| LOG.warn(zkw.prefix("Unable to get create time on znode (" + znode + ")"), e); | ||
| zkw.keeperException(e); | ||
| return -1; |
There was a problem hiding this comment.
Should we return -1 here instead of throwing exception out?
wangxin97-02
force-pushed
the
master
branch
2 times, most recently
from
de4d772 to
b1d16ff
Compare
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Apache9
reviewed
Jun 21, 2024
| @@ -111,6 +114,8 @@ public void addPeer(String peerId, ReplicationPeerConfig peerConfig, boolean ena | |||
| if (!enabled) { | |||
| fs.createNewFile(new Path(peerDir, DISABLED_FILE)); | |||
| } | |||
| write(fs, peerDir, CREATE_TIME_FILE, | |||
There was a problem hiding this comment.
For create time I do not think we need to use the special write method here? As we will never change it...
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
wangxin97-02
commented
Jun 27, 2024
| @@ -111,6 +113,7 @@ public void addPeer(String peerId, ReplicationPeerConfig peerConfig, boolean ena | |||
| if (!enabled) { | |||
| fs.createNewFile(new Path(peerDir, DISABLED_FILE)); | |||
| } | |||
| fs.createNewFile(new Path(peerDir, CREATE_TIME_FILE)); | |||
There was a problem hiding this comment.
I have changed it to just create this file and not add content. So I can use fs.getFileStatus(createTimeFile).getModificationTime() to get peer create time. @Apache9
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the table data needs to be migrated due to the closure of the computer room or the business side needs to have a primary and standby table, we need to create a peer.
By checking the creation time of the peer, we can understand when and what operations were performed, and the impact of these operations on the HBase system status.
When there is a problem with the HBase cluster, such as abnormal data synchronization, performance degradation, etc., displaying the creation time of the peer can help us quickly locate the possible source of the problem. For example, if a peer has a problem shortly after it is created, we can prioritize checking the configuration and status of the peer, optimize the related configuration of replication, etc.
Recording the creation time of the peer helps to audit and track system changes, ensuring that any changes to the replication configuration can be traced. This is very important for meeting data governance, security compliance, and business continuity requirements.