Skip to content

Update to the latest log4j and spotbugs versions#6486

Open
ddanielr wants to merge 10 commits into
apache:2.1from
ddanielr:deps/update-log4j-spotbugs
Open

Update to the latest log4j and spotbugs versions#6486
ddanielr wants to merge 10 commits into
apache:2.1from
ddanielr:deps/update-log4j-spotbugs

Conversation

@ddanielr

Copy link
Copy Markdown
Contributor

Bumps versions and fixes deprecated builder pattern

Bumps versions and fixes deprecated builder pattern
@ddanielr ddanielr added this to the 2.1.7 milestone Jul 21, 2026

@ctubbsii ctubbsii left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The build is still failing. I think it's probably okay to suppress these in 2.1, but in main, we should work to remove the problems spotbugs found, because we do a lot less static stuff in the main branch now, and we can probably do better with regards to these checks there.

ddanielr added 8 commits July 22, 2026 03:46
Added private locks for methods that were synchronized to fix spotbugs
findings
Runnable requires a public run() method so these can't be fixed
Fixed these spotbug findings
The intent of this code is not clear.
This may not be the correct way to do this but will review it at the PR
stage.

@dlmarion dlmarion 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'm a little confused why we are making so many changes if we are also excluding these spotbugs issues.

ThreadPools.watchCriticalScheduledTask(context.getScheduledExecutor().scheduleWithFixedDelay(
new CompactionWatcher(context.getConfiguration()), 10000, 10000, TimeUnit.MILLISECONDS));
watching = true;
public static void startWatching(ServerContext context) {

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.

Does the exclusion not cover this case? Just curious why we are fixing some of these but also have the exclusion.

// remove old settings from zookeeper first, if any
ZooReaderWriter zoo = context.getZooReaderWriter();
synchronized (zooCache) {
synchronized (zooCacheLock) {

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.

Why did this have to change?

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.

3 participants