-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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-3357 [CORE] Internal log messages should be set at DEBUG level instead of INFO #4838
Conversation
Test build #28135 has started for PR 4838 at commit
|
Test build #28135 has finished for PR 4838 at commit
|
Test PASSed. |
@@ -371,7 +371,7 @@ private[spark] class MemoryStore(blockManager: BlockManager, maxMemory: Long) | |||
private def ensureFreeSpace( | |||
blockIdToAdd: BlockId, | |||
space: Long): ResultWithDroppedBlocks = { | |||
logInfo(s"ensureFreeSpace($space) called with curMem=$currentMemory, maxMem=$maxMemory") | |||
logDebug(s"ensureFreeSpace($space) called with curMem=$currentMemory, maxMem=$maxMemory") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one I'd say is fairly important. I've used it somewhat often in production clusters to understand when things are dropping to disk and why.
Chimed in in a few places. I think overall, a good goal is that when we are doing our normal GC of RDD's and broadcasts, we don't want to be so verbose. This cleaning occurs asynchronously and it's confusing for users to see these messages. When dropping blocks due to cache contention though, I'm not so sure we want silence these messages. |
Test build #28145 has started for PR 4838 at commit
|
Test build #28145 has finished for PR 4838 at commit
|
Test PASSed. |
@@ -184,7 +184,7 @@ private[spark] class MemoryStore(blockManager: BlockManager, maxMemory: Long) | |||
val entry = entries.remove(blockId) | |||
if (entry != null) { | |||
currentMemory -= entry.size | |||
logInfo(s"Block $blockId of size ${entry.size} dropped from memory (free $freeMemory)") | |||
logDebug(s"Block $blockId of size ${entry.size} dropped from memory (free $freeMemory)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On this one - do you know if this already gets logged somewhere else if a block is dropped from memory due to contention? It would be good to make sure there is some INFO level logging when a block is dropped due to memory being exceeded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we do have INFO level logging for this up the call chain when drops are blocked due to cache contention:
Might be nice to augment that logging to have information on the size and limit (like this does).
Demote some 'noisy' log messages to debug level. I added a few more, to include everything that gets logged in stanzas like this:
as well as regular messages like
WDYT? good or should some be left alone?
CC @mengxr who suggested some of this.