-
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-21521 Expose master startup status via web UI #4788
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. |
efa91b1
to
297293c
Compare
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.
So what is a task group?
🎊 +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. |
Thanks, @Apache9 . The From my perspective, the Here for the master startup process, we use |
Then please add some comments for TaskGroup about the usage? |
That's the idea, yes. Grouping the tasks is not strictly necessary but in my opinion is cleaner for presentation to operators, and also is similar to how Hadoop designed the startup page for the NameNode. We might want to display the tasks in a group in a list view where each task can be collapsed (probably by default) or expanded. See HDFS-4372 and HDFS-4374, upon which our issue was originally inspired. In https://img-blog.csdnimg.cn/929a86ac0f1748f5af70d27ce14f40ee.png "Loading image" is a task group, and "Loading edits" is another task group. Although not strictly required this kind of grouping can help keep the status page layout clean. If for example in our startup status we have a task group "Deploying tablename" for each table, the individual items in the group would be the TRSP for each region of tablename, and there might be a lot of them, so collapsing the group detail by default would be good. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@@ -1198,6 +1200,8 @@ private void finishActiveMasterInitialization(MonitoredTask status) | |||
} | |||
// Set master as 'initialized'. | |||
setInitialized(true); | |||
startupTaskGroup.markComplete("Initialization successful"); | |||
MonitoredTask afterInitialized = startupTaskGroup.addTask("Progress after master initialized"); |
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.
So why the previous operations are all tasks, but starting from here, we use the same task?
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, we can avoid to add this task after the initialize complete, since it brings some confusion. Thank, @Apache9 . Let me 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. |
💔 -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.
Almost done, thanks. Also noticed a new checkstyle warning
hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/TaskGroup.java
Outdated
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/TaskGroup.java
Show resolved
Hide resolved
hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/TaskMonitor.java
Outdated
Show resolved
Hide resolved
🎊 +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.
ok all set for me. i just noticed one more issue about the updating callers of createStatus to pass the newly inverted boolean value. once you fix those i can approve
hbase-server/src/main/java/org/apache/hadoop/hbase/monitoring/TaskMonitor.java
Outdated
Show resolved
Hide resolved
🎊 +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. |
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
…ache#5021) Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
…5026) Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org>
…ache#5021) Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org> (cherry picked from commit 2dd582c) Change-Id: I7058402f25dd962290fa6976f3701f893cf9bb5f
A new PR after #3667, according to the comments there,