Skip to content

Conversation

@stoty
Copy link
Contributor

@stoty stoty commented Mar 20, 2024

…ut hadoop WIP

@stoty stoty marked this pull request as draft March 20, 2024 11:29
@stoty
Copy link
Contributor Author

stoty commented Mar 20, 2024

Just a very rough draft of how this could be done.
It's probably better to leave this after the test dependencies have been cleaned up.

@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.

@ndimiduk ndimiduk self-requested a review May 17, 2024 14:22
<groupId>org.apache.hadoop</groupId>
<artifactId>hadoop-yarn-api</artifactId>
<version>${hadoop-three.version}</version><exclusions>
<exclusion>
Copy link
Member

Choose a reason for hiding this comment

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

if these are at scope:provided, is it necessary to also have the exclusions list? Can these exclusions be omitted enitrely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They probably can.
I wil test it for the next patch version, and remove them if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@stoty
Copy link
Contributor Author

stoty commented Dec 9, 2024

This includes the refactor and the jax-ws changes that have not yet been merged, but otherwise this is my intended "final" version of the assembly changes.

@stoty stoty marked this pull request as ready for review December 9, 2024 08:34
@Apache9
Copy link
Contributor

Apache9 commented Dec 9, 2024

So we plan to land HBASE-29016 first?

@Apache-HBase

This comment has been minimized.

@stoty
Copy link
Contributor Author

stoty commented Dec 9, 2024

Yes.
It is much easier to do the refactor first, most of changes in HBASE-29016 are required for this to work.
I also plan to merge the jax-ws changes first, so that they don't have to be duplicated either.

I haven't merged HBASE-29016 yet to give you and Nick time to review that.

@stoty
Copy link
Contributor Author

stoty commented Dec 9, 2024

@NihalJain also suggested renaming the new dev classpath generation module, I was also waiting if the name is fine by you, or maybe if you have some different suggestion.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@stoty
Copy link
Contributor Author

stoty commented Dec 9, 2024

This is the diff between the original and byo-hadoop assembly lib dir:

https://issues.apache.org/jira/browse/HBASE-28434?focusedCommentId=17904262&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17904262

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@Apache-HBase

This comment has been minimized.

@stoty
Copy link
Contributor Author

stoty commented Jan 9, 2025

rebased the patch

@stoty
Copy link
Contributor Author

stoty commented Jan 16, 2025

started new CI run

@Apache-HBase

This comment has been minimized.

@Apache-HBase
Copy link

💔 -1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 0s Docker mode activated.
-1 ❌ docker 0m 4s Docker failed to build run-specific yetus/hbase:tp-13752}.
Subsystem Report/Notes
GITHUB PR #5766
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5766/9/console
versions git=2.17.1
Powered by Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache-HBase
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 27s 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.
+0 🆗 shelldocs 0m 0s Shelldocs was not available.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ master Compile Tests _
+0 🆗 mvndep 0m 36s Maven dependency ordering for branch
+1 💚 mvninstall 3m 10s master passed
+1 💚 compile 7m 53s master passed
+1 💚 spotless 0m 44s branch has no errors when running spotless:check.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 15s Maven dependency ordering for patch
+1 💚 mvninstall 2m 51s the patch passed
+1 💚 compile 7m 54s the patch passed
+1 💚 javac 7m 54s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 shellcheck 0m 2s No new issues.
+1 💚 xmllint 0m 1s No new issues.
+1 💚 hadoopcheck 11m 4s Patch does not cause any errors with Hadoop 3.3.6 3.4.0.
+1 💚 spotless 0m 42s patch has no errors when running spotless:check.
_ Other Tests _
+1 💚 asflicense 0m 28s The patch does not generate ASF License warnings.
42m 53s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5766/9/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #5766
Optional Tests dupname asflicense codespell detsecrets shellcheck shelldocs spotless javac xmllint hadoopcheck compile
uname Linux 872c620cfd8e 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 master / 762d719
Default Java Eclipse Adoptium-17.0.11+9
Max. process+thread count 192 (vs. ulimit of 30000)
modules C: hbase-assembly hbase-assembly-byo-hadoop . U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5766/9/console
versions git=2.34.1 maven=3.9.8 shellcheck=0.8.0 xmllint=20913
Powered by Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

</dependency>
<dependency>
<groupId>org.apache.hbase</groupId>
<artifactId>hbase-server</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will pull in bunch of hadoop dependencies? Have you checked the final assembly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I have (though not after the last rebase ).
That's why I've added all the Hadoop dependencies with provided scope, those will remove those dependencies and their transitive dependencies.

I've added a list of the differences between the assemblies (before the last rebase) to the ticket:

https://issues.apache.org/jira/browse/HBASE-28434?focusedCommentId=17904262&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17904262

Copy link
Contributor

@Apache9 Apache9 left a comment

Choose a reason for hiding this comment

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

Tested locally, there are no hadoop jars in the tarball under hbase-assembly-byo-hadoop.

Do we want to publish this tarball in our release too?

@stoty
Copy link
Contributor Author

stoty commented Jan 25, 2025

Thank you.
Yes, I think we should.

@stoty
Copy link
Contributor Author

stoty commented Jan 25, 2025

We need to change the release script for that. right ?

@Apache9
Copy link
Contributor

Apache9 commented Jan 26, 2025

We need to change the release script for that. right ?

Yes, we need to change the release script, I do not think there are any release manager will do release manually now.

Can file a new issue to do the script change.

@stoty
Copy link
Contributor Author

stoty commented Jan 27, 2025

Opened HBASE-29095 for the relase script change.

@stoty stoty merged commit 93b8d7b into apache:master Jan 27, 2025
1 check failed
stoty added a commit to stoty/hbase that referenced this pull request Jan 27, 2025
stoty added a commit that referenced this pull request Jan 28, 2025
…ut hadoop (#5766)

Signed-off-by: Duo Zhang <[email protected]>
(cherry picked from commit 93b8d7b)
fi

#If Hadoop is not specified with HADOOP_HOME, check that the assembly includes Hadoop
if [[ -z "${HADOOP_IN_PATH}" && ! -e "lib/hadoop-common*" ]] ; then
Copy link
Member

Choose a reason for hiding this comment

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

@stoty I just noticed that I'm unable to start hbase in the local dev environment (after mvn install) because of this. How about adding this condition?

diff --git a/bin/hbase b/bin/hbase
index 8f58bf578a..5f1b845110 100755
--- a/bin/hbase
+++ b/bin/hbase
@@ -222,7 +222,7 @@ if [ "$HBASE_DISABLE_HADOOP_CLASSPATH_LOOKUP" != "true" ] ; then
 fi
 
 #If Hadoop is not specified with HADOOP_HOME, check that the assembly includes Hadoop
-if [[ -z "${HADOOP_IN_PATH}" && ! -e "lib/hadoop-common*" ]] ; then
+if [[ $in_dev_env = false && -z "${HADOOP_IN_PATH}" && ! -e "lib/hadoop-common*" ]] ; then
   echo Installation does not contain Hadoop, and HADOOP_HOME does not point to a Hadoop installation.
   echo Specify a compatible Hadoop installation via HADOOP_HOME, or use the HBase assembly variant
   echo that includes Hadoop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, opened HBASE-29106

Copy link
Contributor

Choose a reason for hiding this comment

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

This also breaks the integration test in nightly...

Maybe we should review the condition here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the problem here is that, we should not directly use 'lib/hadoop-common*', since we may execute the script in any directory...

Let's add the ${HBASE_HOME} prefix here... @stoty

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.
Opened #6665 for the fix.

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.

5 participants