Skip to content
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

Backport "HBASE-27814 Add support for dump and process metrics servlet in REST InfoServer" to branch-2.6 #5729

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

NihalJain
Copy link
Contributor

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

@NihalJain NihalJain added the backport This PR is a back port of some issue or issues already committed to master label Feb 29, 2024
@NihalJain
Copy link
Contributor Author

Backports #5215. Sorry for the mess-up in previous PR.

FYI @ndimiduk @bbeaudreault

@NihalJain
Copy link
Contributor Author

Built code locally, extracted assembly, started a local cluster and started HBase REST server.

  1. Verified the landing page has the right page title, all the tabs are visible, hostname is displayed instead of portnumber and no error in page rendering.
  2. Verified /logLevel page is fixed with appropriate tabs, as expected and shown in HBASE-20693
  3. Verified newly added http://localhost:9093/processRest.jsp works as expected
  4. Verified newly added http://localhost:9093/dump page works as expected
  5. Verified all other pages like /logs, /jmx, /conf works as before
  6. Ran postman collection for basic CRUD test for REST at https://gist.github.com/NihalJain/7db91823bd140e41953a88eb3df56eae: All tests PASS
  • Screenshot 2024-02-29 at 9 09 16 PM

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 38s Docker mode activated.
-0 ⚠️ yetus 0m 5s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ branch-2.6 Compile Tests _
+1 💚 mvninstall 3m 2s branch-2.6 passed
+1 💚 compile 0m 17s branch-2.6 passed
+1 💚 shadedjars 5m 44s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 16s branch-2.6 passed
_ Patch Compile Tests _
+1 💚 mvninstall 2m 48s the patch passed
+1 💚 compile 0m 17s the patch passed
+1 💚 javac 0m 17s the patch passed
+1 💚 shadedjars 5m 40s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 14s the patch passed
_ Other Tests _
+1 💚 unit 3m 39s hbase-rest in the patch passed.
24m 4s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5729/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR #5729
Optional Tests javac javadoc unit shadedjars compile
uname Linux 39aad6a109f9 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision branch-2.6 / c8eb184
Default Java Eclipse Adoptium-11.0.17+8
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5729/1/testReport/
Max. process+thread count 1871 (vs. ulimit of 30000)
modules C: hbase-rest U: hbase-rest
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5729/1/console
versions git=2.34.1 maven=3.8.6
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 1m 1s Docker mode activated.
-0 ⚠️ yetus 0m 6s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck
_ Prechecks _
_ branch-2.6 Compile Tests _
+1 💚 mvninstall 2m 50s branch-2.6 passed
+1 💚 compile 0m 20s branch-2.6 passed
+1 💚 shadedjars 5m 2s branch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 19s branch-2.6 passed
_ Patch Compile Tests _
+1 💚 mvninstall 3m 25s the patch passed
+1 💚 compile 0m 22s the patch passed
+1 💚 javac 0m 22s the patch passed
+1 💚 shadedjars 6m 7s patch has no errors when building our shaded downstream artifacts.
+1 💚 javadoc 0m 16s the patch passed
_ Other Tests _
+1 💚 unit 5m 1s hbase-rest in the patch passed.
26m 1s
Subsystem Report/Notes
Docker ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5729/1/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile
GITHUB PR #5729
Optional Tests javac javadoc unit shadedjars compile
uname Linux c010f1fccae8 5.4.0-169-generic #187-Ubuntu SMP Thu Nov 23 14:52:28 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision branch-2.6 / c8eb184
Default Java Temurin-1.8.0_352-b08
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5729/1/testReport/
Max. process+thread count 1838 (vs. ulimit of 30000)
modules C: hbase-rest U: hbase-rest
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5729/1/console
versions git=2.34.1 maven=3.8.6
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Comment
+0 🆗 reexec 0m 38s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ branch-2.6 Compile Tests _
+1 💚 mvninstall 3m 52s branch-2.6 passed
+1 💚 compile 0m 33s branch-2.6 passed
+1 💚 checkstyle 0m 12s branch-2.6 passed
+1 💚 spotless 0m 55s branch has no errors when running spotless:check.
+1 💚 spotbugs 0m 39s branch-2.6 passed
_ Patch Compile Tests _
+1 💚 mvninstall 3m 39s the patch passed
+1 💚 compile 0m 35s the patch passed
-0 ⚠️ javac 0m 35s hbase-rest generated 2 new + 164 unchanged - 2 fixed = 166 total (was 166)
+1 💚 checkstyle 0m 12s the patch passed
+1 💚 whitespace 0m 0s The patch has no whitespace issues.
+1 💚 hadoopcheck 12m 31s Patch does not cause any errors with Hadoop 2.10.2 or 3.3.6.
+1 💚 spotless 0m 51s patch has no errors when running spotless:check.
+1 💚 spotbugs 0m 52s the patch passed
_ Other Tests _
+1 💚 asflicense 0m 11s The patch does not generate ASF License warnings.
27m 34s
Subsystem Report/Notes
Docker ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5729/1/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #5729
Optional Tests dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile
uname Linux 39a6bf738fb0 5.4.0-172-generic #190-Ubuntu SMP Fri Feb 2 23:24:22 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality dev-support/hbase-personality.sh
git revision branch-2.6 / c8eb184
Default Java Eclipse Adoptium-11.0.17+8
javac https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5729/1/artifact/yetus-general-check/output/diff-compile-javac-hbase-rest.txt
Max. process+thread count 78 (vs. ulimit of 30000)
modules C: hbase-rest U: hbase-rest
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5729/1/console
versions git=2.34.1 maven=3.8.6 spotbugs=4.7.3
Powered by Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@NihalJain
Copy link
Contributor Author

NihalJain commented Mar 1, 2024

Hey @ndimiduk could you please review this. This is exact same change as #5215 which you had earlier reviewed.

@ndimiduk
Copy link
Member

ndimiduk commented Mar 1, 2024

Ran postman collection for basic CRUD test for REST at https://gist.github.com/NihalJain/7db91823bd140e41953a88eb3df56eae: All tests PASS

Do we not have this kind of basic functional test in the suite?

Copy link
Member

@ndimiduk ndimiduk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

FYI, I don't usually request review of backport PRs, unless there's something particularly tricky/different between the master and backport changes. In this case, the diffs look basically identical (by my scan), so I would just make the PR to verify the build-bot is happy and move on.

@NihalJain
Copy link
Contributor Author

Do we not have this kind of basic functional test in the suite?

I suppose we do but that must run inside minicluster. I run this by pointing to a node (local/distributed).

FYI, I don't usually request review of backport PRs, unless there's something particularly tricky/different between the master and backport changes.

Ah seems I am being extra cautious. Let me follow this approach going forward.

the diffs look basically identical

They are exactly same here.

Thanks for you review. I will merge this then.

@NihalJain NihalJain merged commit 40a4a97 into apache:branch-2.6 Mar 1, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport This PR is a back port of some issue or issues already committed to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants