-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-22709 Add a chore thread in master to do hbck checking #404
Conversation
💔 -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. |
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is excellent.
Some comments. See what you think.
Lets get this in. I want to add the findings over in HBASE-22723 to your new hbck.jsp.
hbase-server/src/main/jamon/org/apache/hadoop/hbase/tmpl/master/MasterStatusTmpl.jamon
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HbckChecker.java
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HbckChecker.java
Show resolved
Hide resolved
private final Map<String, ServerName> orphanRegionsOnRSSnapshot = new HashMap<>(); | ||
private final List<String> orphanRegionsOnFSSnapshot = new LinkedList<>(); | ||
private final Map<String, Pair<ServerName, List<ServerName>>> inconsistentRegionsSnapshot = | ||
new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
try { | ||
loadRegionsFromFS(); | ||
} catch (IOException e) { | ||
LOG.warn("Faile to load the regions from filesystem", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/Faile/Failed/
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HbckChecker.java
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HbckChecker.java
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HbckChecker.java
Show resolved
Hide resolved
if (hbckChecker != null) { | ||
inconsistentRegions = hbckChecker.getInconsistentRegions(); | ||
orphanRegionsOnRS = hbckChecker.getOrphanRegionsOnRS(); | ||
orphanRegionsOnFS = hbckChecker.getOrphanRegionsOnFS(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, so this stuff could be an hour old?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Date it? See patch in HBASE-22741. It dates the CJ run Report.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a date in the latest code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One other thought, is if there is overlap with existing chores?
Maybe the clusterstatuspublisher?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, also as a follow-on, should make it so user can trigger this chore to run... or perhaps a new shell command which is called hbck_report that runs this chore and the catalogjanitor, etc., to produce all hbck.
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Let me check this. |
This can be done by a new issue. |
The ClusterStatusPublisher only publish version, master and deadservers. Didn't have overlap with this. |
💔 -1 overall
This message was automatically generated. |
Ok on all of the above. Can do aggregation as a follow-on if opportunity. |
💔 -1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few other comments that could be addressed. Missing is dating the Report but can do later I suppose?
This is great.
if (hbckChecker != null) { | ||
inconsistentRegions = hbckChecker.getInconsistentRegions(); | ||
orphanRegionsOnRS = hbckChecker.getOrphanRegionsOnRS(); | ||
orphanRegionsOnFS = hbckChecker.getOrphanRegionsOnFS(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Date it? See patch in HBASE-22741. It dates the CJ run Report.
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Signed-off-by: stack <stack@apache.org>
Signed-off-by: stack <stack@apache.org>
Signed-off-by: stack <stack@apache.org>
Signed-off-by: stack <stack@apache.org>
) Signed-off-by: stack <stack@apache.org> (cherry picked from commit 50e27e2) Change-Id: I1b5d8bfd183147f51f4503e4331d2de23d1656e1
No description provided.