Skip to content

Conversation

@stoty
Copy link
Contributor

@stoty stoty commented Dec 11, 2025

No description provided.

@Apache9
Copy link
Contributor

Apache9 commented Dec 11, 2025

Since these libraries use commons-logging as their logging facade, excluding commons-logging may introduce troubles like ClassNotFoundException? And at least, we can not see the logs of these libraries?

@Apache-HBase

This comment has been minimized.

@stoty
Copy link
Contributor Author

stoty commented Dec 11, 2025

Since these libraries use commons-logging as their logging facade, excluding commons-logging may introduce troubles like ClassNotFoundException?

Not if the the jcl-over-slf4j jar is present on the classpath (which already is). The jcl-over-slf4j jar implements the commons-logging API.

We may have such problems if jcl-over-slf4j is omitted from the classpath.

The Hadoop-common dependency is actually via commons-configuration2, but we don't have that dependency, hence I added the exclusion a level higher.

And at least, we can not see the logs of these libraries?

In my experience commons-logging logs are usually lost, or end up in some unexpeced place because HBase does not include commons-logging configuration.

Using jcl-over-slf4j ensures that the commons-logging messages get piped into SLF4j and from there to Log4j2.

We've done the same in Phoenix some time ago, and tested that the commons-logging components' logs are logged via slf4j->log4j2 (there are very few, we had to work to trigger them)

@Apache-HBase

This comment has been minimized.

@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.
+1 💚 @author 0m 0s The patch does not contain any @author tags.
_ master Compile Tests _
+0 🆗 mvndep 0m 28s Maven dependency ordering for branch
+1 💚 mvninstall 3m 3s master passed
+1 💚 compile 8m 33s master passed
+1 💚 spotless 0m 45s branch has no errors when running spotless:check.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 17s Maven dependency ordering for patch
+1 💚 mvninstall 3m 0s the patch passed
+1 💚 compile 8m 34s the patch passed
+1 💚 javac 8m 34s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 xmllint 0m 0s No new issues.
+1 💚 hadoopcheck 11m 30s Patch does not cause any errors with Hadoop 3.3.6 3.4.1.
+1 💚 spotless 0m 46s patch has no errors when running spotless:check.
_ Other Tests _
+1 💚 asflicense 0m 20s The patch does not generate ASF License warnings.
45m 11s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7539/2/artifact/yetus-general-check/output/Dockerfile
GITHUB PR #7539
Optional Tests dupname asflicense javac codespell detsecrets xmllint hadoopcheck spotless compile
uname Linux bde766358e98 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 / fb26b34
Default Java Eclipse Adoptium-17.0.11+9
Max. process+thread count 190 (vs. ulimit of 30000)
modules C: hbase-diagnostics . U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7539/2/console
versions git=2.34.1 maven=3.9.8 xmllint=20913
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 26s Docker mode activated.
-0 ⚠️ yetus 0m 3s Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --author-ignore-list --blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck
_ Prechecks _
_ master Compile Tests _
+0 🆗 mvndep 0m 23s Maven dependency ordering for branch
+1 💚 mvninstall 3m 12s master passed
+1 💚 compile 2m 8s master passed
+1 💚 javadoc 2m 10s master passed
+1 💚 shadedjars 6m 3s branch has no errors when building our shaded downstream artifacts.
_ Patch Compile Tests _
+0 🆗 mvndep 0m 16s Maven dependency ordering for patch
+1 💚 mvninstall 2m 54s the patch passed
+1 💚 compile 2m 11s the patch passed
+1 💚 javac 2m 11s the patch passed
+1 💚 javadoc 2m 10s the patch passed
+1 💚 shadedjars 5m 59s patch has no errors when building our shaded downstream artifacts.
_ Other Tests _
-1 ❌ unit 263m 27s /patch-unit-root.txt root in the patch failed.
300m 38s
Subsystem Report/Notes
Docker ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7539/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
GITHUB PR #7539
Optional Tests javac javadoc unit shadedjars compile
uname Linux 8b3c6857e3e8 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 / fb26b34
Default Java Eclipse Adoptium-17.0.11+9
Test Results https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7539/2/testReport/
Max. process+thread count 8570 (vs. ulimit of 30000)
modules C: hbase-diagnostics . U: .
Console output https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7539/2/console
versions git=2.34.1 maven=3.9.8
Powered by Apache Yetus 0.15.0 https://yetus.apache.org

This message was automatically generated.

@Apache9
Copy link
Contributor

Apache9 commented Dec 12, 2025

Jcl-over-slf4j is just a commons-logging implementation based on slf4j, so we still need the commons-logging facade? IIRC, commons-logging is mostly like slf4j, usually you will use commons-logging and log4j together in the old time.

So what is the problem of commons-logging dependency here?

@Apache9
Copy link
Contributor

Apache9 commented Dec 12, 2025

Ah, OK, checked the code, seems jcl-over-slf4j contains the class in commons-logging...

@Apache9
Copy link
Contributor

Apache9 commented Dec 12, 2025

Better also ban commons-logging import in the maven enforcer plugin?

@stoty
Copy link
Contributor Author

stoty commented Dec 13, 2025

Better also ban commons-logging import in the maven enforcer plugin?

The commons-logging classes (along with log4j1 and log4j2 ) are already restricted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants