Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-50434][CORE] Make Utils.setLogLevel synchronized #48984

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HyukjinKwon
Copy link
Member

What changes were proposed in this pull request?

This PR proposes to make Utils.setLogLevel synchronized

Why are the changes needed?

There can be a race:

py4j.protocol.Py4JJavaError: An error occurred while calling o20.setLogLevel.
4768: java.util.ConcurrentModificationException
4769	at java.base/java.util.HashMap$ValueSpliterator.forEachRemaining(HashMap.java:1784)
4770	at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762)
4771	at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
4772	at java.base/java.util.WeakHashMap$ValueSpliterator.forEachRemaining(WeakHashMap.java:1217)
4773	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
4774	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
4775	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
4776	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
4777	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
4778	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
4779	at org.apache.logging.log4j.core.LoggerContext.updateLoggers(LoggerContext.java:776)
4780	at org.apache.logging.log4j.core.LoggerContext.updateLoggers(LoggerContext.java:766)
4781	at org.apache.spark.util.Utils$.setLogLevel(Utils.scala:2322)
4782	at org.apache.spark.util.Utils$.setLogLevelIfNeeded(Utils.scala:2331)
4783	at org.apache.spark.SparkContext.setLogLevel(SparkContext.scala:400)
4784	at org.apache.spark.api.java.JavaSparkContext.setLogLevel(JavaSparkContext.scala:675)
4785	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
4786	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
4787	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
4788	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
4789	at py4j.reflection.MethodInvoker.invoke(MethodInvoker.java:244)
4790	at py4j.reflection.ReflectionEngine.invoke(ReflectionEngine.java:374)
4791	at py4j.Gateway.invoke(Gateway.java:282)
4792	at py4j.commands.AbstractCommand.invokeMethod(AbstractCommand.java:132)
4793	at py4j.commands.CallCommand.execute(CallCommand.java:79)
4794	at py4j.ClientServerConnection.waitForCommands(ClientServerConnection.java:182)
4795	at py4j.ClientServerConnection.run(ClientServerConnection.java:106)
4796	at java.base/java.lang.Thread.run(Thread.java:840) 

Does this PR introduce any user-facing change?

I think there should be a rare race condition fixed by this.

How was this patch tested?

Will monitor the build

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions bot added the CORE label Nov 27, 2024
@LuciferYang
Copy link
Contributor

py4j.protocol.Py4JJavaError: An error occurred while calling o107.setLogLevel.
: java.util.ConcurrentModificationException
	at java.base/java.util.HashMap$ValueSpliterator.forEachRemaining(HashMap.java:1784)
	at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:762)
	at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
	at java.base/java.util.WeakHashMap$ValueSpliterator.forEachRemaining(WeakHashMap.java:1217)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
	at org.apache.logging.log4j.core.LoggerContext.updateLoggers(LoggerContext.java:776)
	at org.apache.logging.log4j.core.LoggerContext.updateLoggers(LoggerContext.java:766)
	at org.apache.spark.util.Utils$.setLogLevel(Utils.scala:2322)
	at org.apache.spark.util.Utils$.setLogLevelIfNeeded(Utils.scala:2331)
	at org.apache.spark.SparkContext.setLogLevel(SparkContext.scala:400)
	at org.apache.spark.api.java.JavaSparkContext.setLogLevel(JavaSparkContext.scala:675)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at py4j.reflection.MethodInvoker.invoke(MethodInvoker.java:244)
	at py4j.reflection.ReflectionEngine.invoke(ReflectionEngine.java:374)
	at py4j.Gateway.invoke(Gateway.java:282)
	at py4j.commands.AbstractCommand.invokeMethod(AbstractCommand.java:132)
	at py4j.commands.CallCommand.execute(CallCommand.java:79)
	at py4j.ClientServerConnection.waitForCommands(ClientServerConnection.java:182)
	at py4j.ClientServerConnection.run(ClientServerConnection.java:106)
	at java.base/java.lang.Thread.run(Thread.java:840)

seems the problem still exists. There may another thread traversing org.apache.logging.log4j.core.util.internal.InternalLoggerRegistry#loggerRefByNameByMessageFactory, but its entry point is not Utils.setLogLevel.

@HyukjinKwon
Copy link
Member Author

yeah lemme take a look

@HyukjinKwon HyukjinKwon marked this pull request as draft November 27, 2024 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants