Skip to content

HBASE-29037 Backport missing UI patches to branch-2 - #6551

Merged
NihalJain merged 4 commits into
apache:branch-2from
PDavid:HBASE-29037-UI-missing-patches-branch-2
Dec 24, 2024
Merged

HBASE-29037 Backport missing UI patches to branch-2#6551
NihalJain merged 4 commits into
apache:branch-2from
PDavid:HBASE-29037-UI-missing-patches-branch-2

Conversation

@PDavid

Copy link
Copy Markdown
Contributor

Cherry-picked the following patches which were missing on branch-2 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.

songxincunand others added 4 commits December 18, 2024 08:33
Signed-off-by: Guangxu Cheng <gxcheng@apache.org>
(cherry picked from commit 9ad16aa)
…empty start key/end key (apache#2955)
Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Pankaj Kumar<pankajkumar@apache.org>
(cherry picked from commit 157200e)
…procedure.jsp while Master is initializing (apache#6152)
Signed-off-by: Duo Zhang <zhangduo@apache.org>
(cherry picked from commit 3caaf2d)
…elds before submit
Signed-off-by: tedyu <yuzhihong@gmail.com>
(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.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 47sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗codespell0m 0scodespell was not available.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
_ branch-2 Compile Tests _
+1 💚mvninstall2m 39sbranch-2 passed
+1 💚spotless0m 42sbranch has no errors when running spotless:check.
_ Patch Compile Tests _
+1 💚mvninstall2m 36sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚spotless0m 41spatch has no errors when running spotless:check.
_ Other Tests _
+1 💚asflicense0m 11sThe patch does not generate ASF License warnings.
8m 43s
SubsystemReport/Notes
DockerClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6551/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR#6551
Optional Testsdupname asflicense javac codespell detsecrets spotless
unameLinux 7874217ca65c 5.4.0-200-generic #220-Ubuntu SMP Fri Sep 27 13:19:16 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionbranch-2 / efc79d0
Default JavaEclipse Adoptium-11.0.23+9
Max. process+thread count79 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6551/2/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 50sDocker mode activated.
-0 ⚠️yetus0m 5sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
_ Prechecks _
_ branch-2 Compile Tests _
+1 💚mvninstall2m 54sbranch-2 passed
+1 💚javadoc0m 25sbranch-2 passed
_ Patch Compile Tests _
+1 💚mvninstall2m 49sthe patch passed
+1 💚javadoc0m 24sthe patch passed
_ Other Tests _
+1 💚unit185m 5shbase-server in the patch passed.
196m 43s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6551/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile
GITHUB PR#6551
Optional Testsjavac javadoc unit
unameLinux fd5cfbf53ae2 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionbranch-2 / efc79d0
Default JavaEclipse Adoptium-11.0.23+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6551/2/testReport/
Max. process+thread count4134 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6551/2/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 50sDocker mode activated.
-0 ⚠️yetus0m 5sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
_ Prechecks _
_ branch-2 Compile Tests _
+1 💚mvninstall2m 44sbranch-2 passed
+1 💚javadoc0m 25sbranch-2 passed
_ Patch Compile Tests _
+1 💚mvninstall2m 24sthe patch passed
+1 💚javadoc0m 24sthe patch passed
_ Other Tests _
+1 💚unit186m 54shbase-server in the patch passed.
198m 17s
SubsystemReport/Notes
DockerClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6551/2/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile
GITHUB PR#6551
Optional Testsjavac javadoc unit
unameLinux 2b7d012ee11d 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 toolmaven
Personalitydev-support/hbase-personality.sh
git revisionbranch-2 / efc79d0
Default JavaTemurin-1.8.0_412-b08
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6551/2/testReport/
Max. process+thread count4150 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6551/2/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 23sDocker mode activated.
-0 ⚠️yetus0m 5sUnprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
_ Prechecks _
_ branch-2 Compile Tests _
+1 💚mvninstall2m 35sbranch-2 passed
+1 💚javadoc0m 28sbranch-2 passed
_ Patch Compile Tests _
+1 💚mvninstall2m 39sthe patch passed
+1 💚javadoc0m 27sthe patch passed
_ Other Tests _
+1 💚unit205m 4shbase-server in the patch passed.
215m 53s
SubsystemReport/Notes
DockerClientAPI=1.47 ServerAPI=1.47 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6551/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR#6551
Optional Testsjavac javadoc unit
unameLinux a81bfaf70127 5.4.0-200-generic #220-Ubuntu SMP Fri Sep 27 13:19:16 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitydev-support/hbase-personality.sh
git revisionbranch-2 / efc79d0
Default JavaEclipse Adoptium-17.0.11+9
Test Resultshttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6551/2/testReport/
Max. process+thread count4660 (vs. ulimit of 30000)
modulesC: hbase-server U: hbase-server
Console outputhttps://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-6551/2/console
versionsgit=2.34.1 maven=3.9.8
Powered byApache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@NihalJain

Copy link
Copy Markdown
Contributor

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

@PDavid

Copy link
Copy Markdown
ContributorAuthor

Hi @NihalJain,
Many thanks, I think this is now ready to review.

@PDavid
PDavid marked this pull request as ready for review December 19, 2024 08:40
@NihalJain

NihalJain commented Dec 20, 2024

Copy link
Copy Markdown
Contributor

Reviewing commit by commit:

  1. HBASE-24624 Optimize table.jsp code: TODO
    • This change is huge wrt to original PR, @PDavid could you summarise what all we changed here? Is there a whitespace issue? Mind fixing it? Or are you doing this to align with Master?
  2. HBASE-25402 Sorting order by start key or end key is not considering empty start key/end key -> LGTM
  3. HBASE-28778 NPE may occur when opening master-status or table.jsp or procedure.jsp while Master is initializing: -> LGTM
    • Apparently this was already backported with original ticket, we just missed a few files back then!
  4. HBASE-20452 Master UI: Table merge button should validate required fields before submit -> LGTM

Also could you please summarise how the changes have been tested.

@Apache9Apache9 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've tried to build a tarball with the PR and started a mini cluster locally, and run LTT to load some data, the page looks good.

Let's get this in and move on.

Thanks @PDavid !

@NihalJain

Copy link
Copy Markdown
Contributor

Thanks @Apache9 for testing out. We can look if anything needs to be done for whitespace later. Skimmed commit 1, LGTM Merging this to codebase.

@PDavid

Copy link
Copy Markdown
ContributorAuthor

Hi @NihalJain and @Apache9,

Many thanks for your reviews and tests. 🙏

Sorry, I was completely off in the holiday season.

  1. HBASE-24624 Optimize table.jsp code: TODO

    • This change is huge wrt to original PR, @PDavid could you summarise what all we changed here? Is there a whitespace issue? Mind fixing it? Or are you doing this to align with Master?

The motivation behind these backport UI changes PR-s (and this specific backport you mention) is to align with master branch so that the Boostrap upgrade can be backported more cleanly.

Here the problem was that - if you check the history of table.jsp on master branch and on branch-2 - that this "Optimize table.jsp code" patch was missing from branch-2 but the newer patches were applied (backported) on branch-2. This made table.jsp quite different than it was when "Optimize table.jsp code" patch was merged to master branch.

So when I backported this change I was extra careful to not to "undo" the changes which newer backported patches did.

Also could you please summarise how the changes have been tested.

Sure. I built the project in dev mode with mvn clean install -DskipTests -Dhadoop.profile=3.0 then started HBase locally with bin/start-hbase.sh in standalone mode. Created some tables with some rows and checked the Master UI, region server UI.
Started Thrift server (bin/hbase thrift start -p 8081) and REST server (bin/hbase rest start -p 8080) and checked their UI respectively.

To properly verify the REST UI I had to copy the static resources folder under hbase-rest/src/main/resources/hbase-webapps folder because of the REST UI is broken (static resources are not loaded) https://issues.apache.org/jira/browse/HBASE-28983 issue - but this change I did not committed as there is a separate Jira.

Hope this answers your question.

Any suggestions maybe in regards to testing?

@NihalJain

Copy link
Copy Markdown
Contributor

Thanks @PDavid for the response, I think we are good here.

@PDavid
PDavid deleted the HBASE-29037-UI-missing-patches-branch-2 branch January 6, 2025 11:02
mokai87 pushed a commit to mokai87/hbase that referenced this pull request Aug 7, 2025
* HBASE-24624 Optimize table.jsp code (apache#1963)
Signed-off-by: Guangxu Cheng <gxcheng@apache.org>
(cherry picked from commit 9ad16aa)
* HBASE-25402 Sorting order by start key or end key is not considering empty start key/end key (apache#2955)
Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Pankaj Kumar<pankajkumar@apache.org>
(cherry picked from commit 157200e)
* HBASE-28778 NPE may occur when opening master-status or table.jsp or procedure.jsp while Master is initializing (apache#6152)
Signed-off-by: Duo Zhang <zhangduo@apache.org>
(cherry picked from commit 3caaf2d)
* HBASE-20452 Master UI: Table merge button should validate required fields before submit
Signed-off-by: tedyu <yuzhihong@gmail.com>
(cherry picked from commit 6ce1136)
---------
Co-authored-by: xincunSong <365724453@qq.com>
Co-authored-by: Akshay Sudheer <74921542+AkshayTSudheer@users.noreply.github.com>
Co-authored-by: Peng Lu <lupeng_nwpu@qq.com>
Co-authored-by: Nihal Jain <nihaljain.cs@gmail.com>
Signed-off-by: Duo Zhang <zhangduo@apache.org>
Signed-off-by: Nihal Jain <nihaljain.cs@gmail.com>
Sign up for freeto 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.

7 participants

@PDavid@Apache-HBase@NihalJain@Apache9@songxincun@AkshayTSudheer@guluo2016