[SPARK-28929][CORE] Spark Logging level should be INFO instead of DEBUG in Executor Plugin API #25634
Conversation
| val pluginNames = conf.get(EXECUTOR_PLUGINS) | ||
| if (pluginNames.nonEmpty) { | ||
| logDebug(s"Initializing the following plugins: ${pluginNames.mkString(", ")}") | ||
| logInfo(s"Initializing the following plugins: ${pluginNames.mkString(", ")}") |
There was a problem hiding this comment.
Hi, @iRakson . At the PR description, could you describe your reason why this should be INFO?
There was a problem hiding this comment.
It sounds reasonable to use logInfo for this message. The message is only shown once for each executor. Debug level might be too low here.
|
ok to test |
|
LGTM |
|
Got it. However, this PR intentionally violates our templates. Our template consists of four parts and the second part is the following. The author had better follow the official Apache Spark guideline.
Please update the PR description before you merge this, @gatorsmile . Thanks~ |
|
Test build #109988 has finished for PR 25634 at commit
|
|
Yes, please keep the items in the PR description ... |
|
@gatorsmile @dongjoon-hyun PR description has been modified. Please review. |
|
(Pending tests of course, and Jenkins is offline right now) |
|
Test build #4867 has finished for PR 25634 at commit
|
|
Although the failure is irrelevant to this, I tested this with the failed test cases. I'll merge this to master. Thank you, @iRakson , @gatorsmile , @HyukjinKwon and @srowen . |
6 participants
What changes were proposed in this pull request?
Log levels in Executor.scala are changed from DEBUG to INFO.
Why are the changes needed?
Logging level DEBUG is too low here. These messages are simple acknowledgement for successful loading and initialization of plugins. So its better to keep them in INFO level.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Manually tested.