Reverting log4j changes from PR 11702

[mcvsubbu]

Moving to later versions of log4j causes thread contention, and multiple production systems have been affected. Backing out to use the older version of log4j

[mcvsubbu]

cc: @gortiz

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 34.85%. Comparing base (8e8bd32) to head (ab48e78).
⚠️ Report is 3257 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (8e8bd32) and HEAD (ab48e78). Click for more details.

HEAD has 10 uploads less than BASE

Flag	BASE (8e8bd32)	HEAD (ab48e78)
unittests	5	3
temurin	14	12
java-21	10	8
skip-bytebuffers-true	4	3
skip-bytebuffers-false	7	6
unittests2	2	0

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #11903       +/-   ##
=============================================
- Coverage     61.40%   34.85%   -26.56%     
+ Complexity     1147      945      -202     
=============================================
  Files          2375     2299       -76     
  Lines        128519   124784     -3735     
  Branches      19849    19295      -554     
=============================================
- Hits          78916    43490    -35426     
- Misses        43900    78255    +34355     
+ Partials       5703     3039     -2664     

Flag	Coverage Δ
custom-integration1	<0.01% <ø> (ø)
integration	<0.01% <ø> (ø)
integration1	<0.01% <ø> (ø)
integration2	0.00% <ø> (ø)
java-11	34.81% <ø> (+<0.01%)	⬆️
java-21	34.72% <ø> (-26.56%)	⬇️
skip-bytebuffers-false	34.83% <ø> (-26.56%)	⬇️
skip-bytebuffers-true	34.70% <ø> (-26.56%)	⬇️
temurin	34.85% <ø> (-26.56%)	⬇️
unittests	46.64% <ø> (-14.76%)	⬇️
unittests1	46.64% <ø> (+<0.01%)	⬆️
unittests2	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gortiz]

The reason to move to newer log4j2 versions is that Spark 3.5 requires slf4j2. This library introduces binary incompatibility changes and our current maven config silences these requirements, so as you can see we end up having an error in runtime (detected by the tests).

In order to support slf4j, log4j2 provides different log4j-slf4jXX-impl because slf4j breaks binary compatibility quite often. The first version log4j2 that provides a log4j-slf4j2-impl adaptor is 2.19.0. One option would be to use that instead of 2.20.0 used in my PR.

The other alternative would be to keep log4j2 in v2.17.1 (although it was released almost 2 years ago!) and try to use as adaptor log4j-slf4j18, which in the release notes says it should support slf4j2 v2.0.0-ALPHA (link to release notes). I didn't test this, so I'm not sure if it is a viable alternative or not.

The latest alternative I can imagine would be to properly analyze the test that fail. Although in the log I can see the error is caught by the recently added Gradle plugin, IIRC the problem was in SparkSegmentGenerationJobRunnerTest.setup() when creating a new SparkContext. It may be possible to skip calling slf4j there, although this is the alternative I like the least. Our pipeline is very weak detecting dependency issues and this would just silence a possible error in production.

I was also looking for the lock contention issue in log4j2, but I didn't find any report on that. Is there a ticket in log4j2 I can look for? In case it doesn't, it would be great to report the issue there. Given how many projects use log4j2, fixing a performance issue there would be great for the community.

[Jackie-Jiang]

@mcvsubbu I want to follow up on this issue: Has the problem been solved? Did you take any workaround?

[mcvsubbu]

@mcvsubbu I want to follow up on this issue: Has the problem been solved? Did you take any workaround?

No, we pin the version of log4j in our deployments.

[Jackie-Jiang]

@mcvsubbu Any idea what could have caused the contention? Want to see if there is a way to fix this. (We didn't observe any contention though)

[Jackie-Jiang]

More details in #11909