-
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-27814 Add support for dump and process metrics servlet in REST … #5215
Conversation
Started a local cluster and started HBase REST server.
Also tried the patch on a real cluster by building against 2.5.x and verified above points. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/RESTServer.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. |
Hi could someone please have a look at this, if familiar with REST web UI related code? I have attached screenshots post testing in the PR as well. |
Hi @Reidddddd, are you familiar with web UI related code? If so, could you please help me with review for this? |
🎊 +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.
Just some minor comments. Nice addition. Sorry to keep you waiting on the review.
hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/RESTDumpServlet.java
Show resolved
Hide resolved
hbase-rest/src/main/java/org/apache/hadoop/hbase/rest/RESTDumpServlet.java
Show resolved
Hide resolved
I will rebase the code, re run all tests locally and if no issues found, will push this change as is tomorrow. Action items based on review: https://issues.apache.org/jira/browse/HBASE-28300, https://issues.apache.org/jira/browse/HBASE-28299 will be taken up as follow up. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
380d029
to
52926d0
Compare
Rebased code to let CI build the change again. Also will retest the change locally. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Reverfied all of this. I also wrote a simple postman collection for basic CRUD test for REST at https://gist.github.com/NihalJain/7db91823bd140e41953a88eb3df56eae Can we keep this inside test resources of hbase REST? Might be useful for others as well. Please let me know what you think. |
@bbeaudreault please let me know if you want this in 2.6. |
…InfoServer (#5215) Other changes: - Ensure info server stops during stop() - Extract header and footer. This would fix the log level page layout for rest web UI (See HBASE-20693) - Add hostname in the landing page instead of just port similar to other web UIs Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> (cherry picked from commit a683fcf)
…InfoServer (#5215) Other changes: - Ensure info server stops during stop() - Extract header and footer. This would fix the log level page layout for rest web UI (See HBASE-20693) - Add hostname in the landing page instead of just port similar to other web UIs Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> (cherry picked from commit a683fcf)
You mean keep the postman config definition? Can it be used for an automated testing? I've used postman before, for API development and manual testing, but I've never tried to wire it up to a maven test phase. I am in favor of improving our test coverage if it's lacking. |
Thanks a lot for seeing this one through, and for taking the time to file follow-up issues ! |
I will pull this in branch-2.6, it is unreleased yet. And its best to keep code in sync in branch-2.6+. Also I plan to do more fixes around this in future (for eg @ndimiduk review comments), all of which may go over same code and hence we would have to skip them for branch-2.6, which is a bad idea. |
…InfoServer
Other changes: