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

HBASE-29028 Backport missing UI patches to branch-2.5 #6542

Merged

Conversation

PDavid
Copy link
Contributor

@PDavid PDavid commented Dec 13, 2024

Cherry-picked the following patches which were missing on branch-2.5 branch. Most of these applied as a clean cherry-pick (without conflicts). Unfortunately there were some which had conflicts and I manually resolved them.
Each commit message contains from which commit it was cherry-picked from.

Is it OK to have multiple patches in one PR like this or would you prefer having a separate PR for each commit?

lucakovacs and others added 4 commits December 12, 2024 15:36
…e#4833)

Signed-off-by: Andor Molnar <[email protected]>
Signed-off-by: Balazs Meszaros <[email protected]>
(cherry picked from commit dffc8e0)
…InfoServer (apache#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 <[email protected]>
(cherry picked from commit a683fcf)
Signed-off-by: Wellington Chevreuil <[email protected]>
Signed-off-by: Bharath Vissapragada <[email protected]>
Signed-off-by: Viraj Jasani <[email protected]>
(cherry picked from commit 82e155e)
…he#5732)

- Fixes the way logLevel page renders in UI

Signed-off-by: Nick Dimiduk <[email protected]>
(cherry picked from commit ede4ccd)
@PDavid PDavid changed the title HBASE-29028 Backport missing UI patches to branch-2.x HBASE-29028 Backport missing UI patches to branch-2.5 Dec 13, 2024
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

songxincun and others added 6 commits December 16, 2024 10:10
Signed-off-by: Guangxu Cheng <[email protected]>
(cherry picked from commit 9ad16aa)
…empty start key/end key (apache#2955)

Signed-off-by: Duo Zhang <[email protected]>
Signed-off-by: Pankaj Kumar<[email protected]>
(cherry picked from commit 157200e)
… table page (apache#4793)

Co-authored-by: zhengsicheng <[email protected]>
Signed-off-by: Duo Zhang <[email protected]>
(cherry picked from commit eb6b274)
…procedure.jsp while Master is initializing (apache#6152)

Signed-off-by: Duo Zhang <[email protected]>
(cherry picked from commit 3caaf2d)
…pache#5620)

Co-authored-by: Haosen Chen <[email protected]>
Signed-off-by: Duo Zhang <[email protected]>
(cherry picked from commit e3a0174)
…elds before submit

Signed-off-by: tedyu <[email protected]>
(cherry picked from commit 6ce1136)
@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@PDavid PDavid marked this pull request as ready for review December 18, 2024 07:23
@NihalJain
Copy link
Contributor

Hi @PDavid please let me know once this PR is ready for review

@PDavid
Copy link
Contributor Author

PDavid commented Dec 19, 2024

Hi @NihalJain,
Many thanks, I think this is now ready to review. Some tests failed in the PR build but I think they are not related.

@NihalJain
Copy link
Contributor

NihalJain commented Dec 24, 2024

Tried the change, they work fine for me:

  • Master: LGTM, except navbar all look good. Again navbar should not be issue as bootstrap bump will fix it eventually
Screenshot 2024-12-24 at 12 46 46 PM
  • Rest UI: LGTM
Screenshot 2024-12-24 at 12 46 29 PM
  • Thrift UI: LGTM
Screenshot 2024-12-24 at 12 46 35 PM

Copy link
Contributor

@NihalJain NihalJain left a comment

Choose a reason for hiding this comment

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

We may need to drop HBASE-27406 Make /prometheus endpoint accessible from HBase UI (#4833) as the feature is not even supported by backend.
Trying to open /prometheus give 404 with this change.

Screenshot 2024-12-24 at 12 31 09 PM

@PDavid
Copy link
Contributor Author

PDavid commented Jan 6, 2025

Hi @NihalJain,

Many thanks for testing the changes. 👍

We may need to drop HBASE-27406 Make /prometheus endpoint accessible from HBase UI (#4833) as the feature is not even supported by backend. Trying to open /prometheus give 404 with this change.

Screenshot 2024-12-24 at 12 31 09 PM

Very good catch, thanks. I was not aware and somehow did not found this. 🤦‍♂️ I'll have a look and fix this.

Would it work if we keep the "Metrics" dropdown but remove the Prometheus links? So we would only have the "JMX" and "JMX with description" links there.

What do you think?

@NihalJain
Copy link
Contributor

Would it work if we keep the "Metrics" dropdown but remove the Prometheus links? So we would only have the "JMX" and "JMX with description" links there.

Sounds good to me!

@PDavid PDavid marked this pull request as draft January 6, 2025 13:55
as the feature (HBASE-20904) is not even supported by backend.
@PDavid
Copy link
Contributor Author

PDavid commented Jan 6, 2025

OK, now the Prometheus links are removed.

Master UI

image

Region Server UI

image

REST Server UI

image

Thrift Server UI

image

@NihalJain
Copy link
Contributor

NihalJain commented Jan 7, 2025

Dropped a mail on dev thread to decide on whether we want to take this and other related changes in before the upcoming release. https://lists.apache.org/thread/86ds4dxprvyot243g3jg0ztxzjn5btrq

FYI @PDavid

CC: #6525

@PDavid PDavid marked this pull request as ready for review January 7, 2025 09:26
@Apache-HBase
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 48s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 codespell 0m 0s codespell was not available.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
+1 💚 hbaseanti 0m 0s Patch does not have any anti-patterns.
_ branch-2.5 Compile Tests _
+0 🆗 mvndep 0m 13s Maven dependency ordering for branch
+1 💚 mvninstall 2m 58s branch-2.5 passed
+1 💚 compile 3m 36s branch-2.5 passed
+1 💚 checkstyle 1m 14s branch-2.5 passed
+1 💚 spotbugs 2m 57s branch-2.5 passed
-1 ❌ spotless 0m 44s branch has 1 errors when running spotless:check, run spotless:apply to fix.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 14s Maven dependency ordering for patch
+1 💚 mvninstall 2m 44s the patch passed
+1 💚 compile 3m 38s the patch passed
+1 💚 javac 3m 38s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 1m 13s the patch passed
+1 💚 spotbugs 3m 22s the patch passed
+1 💚 hadoopcheck 20m 34s Patch does not cause any errors with Hadoop 2.10.2 or 3.2.4 3.3.6 3.4.0.
+1 💚 spotless 0m 44s patch has no errors when running spotless:check.
_ Other Tests _
+1 💚 asflicense 0m 27s The patch does not generate ASF License warnings.
47m 32s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6542/3/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #6542
Optional Tests dupname asflicense javac spotbugs checkstyle codespell detsecrets compile hadoopcheck hbaseanti spotless
uname Linux 169095e7f19e 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.5 / bb074da
Default Java Eclipse Adoptium-11.0.23+9
spotless https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6542/3/artifact/yetus-general-check/output/branch-spotless.txt
Max. process+thread count 78 (vs. ulimit of 30000)
modules C: hbase-server hbase-thrift hbase-rest U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6542/3/console
versions git=2.34.1 maven=3.9.8 spotbugs=4.7.3
Powered by Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@PDavid
Copy link
Contributor Author

PDavid commented Jan 7, 2025

The spotless check itself was successful in the CI build but there was some other error:

INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  39.461 s
[INFO] Finished at: 2025-01-07T13:29:55Z
[INFO] ------------------------------------------------------------------------
[ERROR] An internal error occurred during: "Periodic workspace save.".
java.lang.IllegalStateException: Job manager has been shut down.
    at org.eclipse.core.internal.jobs.JobManager.schedule (JobManager.java:1295)
    at org.eclipse.core.internal.jobs.InternalJob.schedule (InternalJob.java:385)
    at org.eclipse.core.runtime.jobs.Job.schedule (Job.java:684)
    at org.eclipse.core.internal.events.AutoBuildJob.build (AutoBuildJob.java:110)
    at org.eclipse.core.internal.events.BuildManager.endTopLevel (BuildManager.java:604)
    at org.eclipse.core.internal.resources.Workspace.endOperation (Workspace.java:1518)
    at org.eclipse.core.internal.resources.SaveManager.save (SaveManager.java:1246)
    at org.eclipse.core.internal.resources.SaveManager.save (SaveManager.java:1143)
    at org.eclipse.core.internal.resources.DelayedSnapshotJob.run (DelayedSnapshotJob.java:55)
    at org.eclipse.core.internal.jobs.Worker.run (Worker.java:63)

Also run spotless check locally and was successful.

Copy link
Contributor

@apurtell apurtell left a comment

Choose a reason for hiding this comment

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

I see the branch-2 PR has been merged already, and that presumably the branch-2.6 (#6550) will be merged soon. This can go in to branch-2.5 after that. @NihalJain

@NihalJain
Copy link
Contributor

We already have a go ahead for this; thanks to @Apache9 and @apurtell on mail thread.
Hi, @PDavid Please let me know if this is good to merge today!

@PDavid
Copy link
Contributor Author

PDavid commented Jan 8, 2025

We already have a go ahead for this; thanks to @Apache9 and @apurtell on mail thread. Hi, @PDavid Please let me know if this is good to merge today!

Hi @NihalJain and @apurtell,

Many thanks for the reviews. I think this is ready to be merged.

@NihalJain NihalJain merged commit 526bad7 into apache:branch-2.5 Jan 8, 2025
1 check failed
@PDavid PDavid deleted the HBASE-29028-UI-missing-patches-branch-2.5 branch January 8, 2025 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.