[SPARK-17649][Core]Log how many Spark events got dropped in LiveListenerBus #15220
Conversation
|
/cc @yhuai |
| // and we will try again. | ||
| if (droppedEventsCounter.compareAndSet(droppedEvents, 0)) { | ||
| lastReportTimestamp = System.currentTimeMillis() | ||
| logWarning(s"Dropped $droppedEvents SparkListenerEvents") |
There was a problem hiding this comment.
Should we also log the lastReportTimestamp?
There was a problem hiding this comment.
Should we also log the lastReportTimestamp?
I don't think so since the log itself has the time info already.
| // Don't log too frequently | ||
| if (System.currentTimeMillis() - lastReportTimestamp >= 60 * 1000) { | ||
| var droppedEvents = droppedEventsCounter.get | ||
| while (droppedEvents > 0) { |
There was a problem hiding this comment.
I do not quite get the purpose of this while loop. Should we just move forward if droppedEventsCounter.compareAndSet(droppedEvents, 0) fail? When compareAndSet fails, there must be another thread that has triggered the log, right?
There was a problem hiding this comment.
I do not quite get the purpose of this while loop. Should we just move forward if droppedEventsCounter.compareAndSet(droppedEvents, 0) fail? When compareAndSet fails, there must be another thread that has triggered the log, right?
The loop is not necessary. Updated. Also modified to check droppedEvents > 0 first so that don't need to call System.currentTimeMillis() in most of cases.
|
Test build #65836 has finished for PR 15220 at commit
|
| /** A counter for dropped events. It will be reset every time we log it. */ | ||
| private val droppedEventsCounter = new AtomicLong(0L) | ||
|
|
||
| /** When `droppedEventsCounter` was logged last time. */ |
There was a problem hiding this comment.
Mention that it stores millis?
|
LGTM. Just left one comment to make the comment a little bit more clear. |
|
Test build #65843 has finished for PR 15220 at commit
|
|
Test build #65847 has finished for PR 15220 at commit
|
|
Thanks! Merging to master / 2.0. I will submit a patch for 1.6. |
|
Test build #65851 has finished for PR 15220 at commit
|
…enerBus ## What changes were proposed in this pull request? Log how many Spark events got dropped in LiveListenerBus so that the user can get insights on how to set a correct event queue size. ## How was this patch tested? Jenkins Author: Shixiong Zhu <[email protected]> Closes #15220 from zsxwing/SPARK-17649. (cherry picked from commit bde85f8) Signed-off-by: Shixiong Zhu <[email protected]>
…nousListenerBus (branch 1.6) ## What changes were proposed in this pull request? Backport #15220 to 1.6. ## How was this patch tested? Jenkins Author: Shixiong Zhu <[email protected]> Closes #15226 from zsxwing/SPARK-17649-branch-1.6.
…nousListenerBus (branch 1.6) ## What changes were proposed in this pull request? Backport apache#15220 to 1.6. ## How was this patch tested? Jenkins Author: Shixiong Zhu <[email protected]> Closes apache#15226 from zsxwing/SPARK-17649-branch-1.6. (cherry picked from commit 7aded55)
3 participants
What changes were proposed in this pull request?
Log how many Spark events got dropped in LiveListenerBus so that the user can get insights on how to set a correct event queue size.
How was this patch tested?
Jenkins