spark icon indicating copy to clipboard operation
spark copied to clipboard

[SPARK-40511][BUILD][CORE] Upgrade slf4j to 2.0.2

Open LuciferYang opened this issue 2 years ago • 6 comments

What changes were proposed in this pull request?

This pr aims to upgrade slf4j from 1.7.36 to 2.0.x, the main change as follows:

  1. Upgrade slf4j version from 1.7.36 to 2.0.x
  2. Use log4j-slf4j2-impl instead of log4j-slf4j-impl refer to https://logging.apache.org/log4j/2.x/log4j-slf4j-impl/index.html
  3. Adapt to the implementation of Logging.isLog4j2(), slf4j2 uses SLF4JServiceProvider instead of StaticLoggerBinder

Why are the changes needed?

slf4j 1.7.36 is no longer actively developed, and the current stable and actively developed version of slf4j is 2.0.x.

  • https://www.slf4j.org/news.html
  • https://www.slf4j.org/faq.html#changesInVersion200
  • https://www.slf4j.org/download.html

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GitHub Actions

LuciferYang avatar Sep 09 '22 06:09 LuciferYang

Test API compatibility first

LuciferYang avatar Sep 09 '22 06:09 LuciferYang

close and wait log4j 2.19.x

LuciferYang avatar Sep 09 '22 09:09 LuciferYang

@LuciferYang https://search.maven.org/artifact/org.apache.logging.log4j/log4j-slf4j2-impl has been released!

martin-g avatar Sep 19 '22 06:09 martin-g

Thanks @martin-g

LuciferYang avatar Sep 19 '22 06:09 LuciferYang

reopen this due to log4j 2.19.0 released and Spark already upgrade to use this version, test first

LuciferYang avatar Sep 20 '22 06:09 LuciferYang

friendly ping @viirya @HyukjinKwon @dongjoon-hyun @srowen @martin-g

This pr try to upgrade slf4j from 1.x to 2.x, but I don't think it is an appropriate time to upgrade it now due to slf4j 2.0.x still released frequently, so I will keep this pr as draft for a period of time and continuous testing it.

LuciferYang avatar Sep 21 '22 06:09 LuciferYang

so I will keep this pr as draft for a period of time and continuous testing it.

So this is changed?

viirya avatar Sep 21 '22 17:09 viirya

so I will keep this pr as draft for a period of time and continuous testing it.

So this is changed?

may be a mistake, re-change to draft

LuciferYang avatar Sep 21 '22 17:09 LuciferYang

mvn test Java8 + hadoop-2 profile with this pr , all test passed

LuciferYang avatar Sep 28 '22 09:09 LuciferYang

2.0.3 released, let us wait more time

LuciferYang avatar Sep 29 '22 11:09 LuciferYang

mvn test Java17 + hadoop-3 profile with this pr , all test passed

LuciferYang avatar Oct 02 '22 13:10 LuciferYang

Is this ready to merge?

srowen avatar Oct 04 '22 13:10 srowen

I think the code is ready for review,but I'm not sure whether the slf4j will continue to release the versions frequently recently, I set this one to ready for review first.

LuciferYang avatar Oct 04 '22 13:10 LuciferYang

should we merge this one first ?

LuciferYang avatar Oct 06 '22 05:10 LuciferYang

Yep. Thank you, @LuciferYang and all. Merged to master.

dongjoon-hyun avatar Oct 06 '22 07:10 dongjoon-hyun

Thanks @martin-g @dongjoon-hyun @HyukjinKwon @srowen @viirya @itholic

LuciferYang avatar Oct 06 '22 07:10 LuciferYang