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-17649][CORE] Log how many Spark events got dropped in AsynchronousListenerBus (branch 1.6) #15226

Closed
wants to merge 1 commit into from

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Sep 24, 2016

What changes were proposed in this pull request?

Backport #15220 to 1.6.

How was this patch tested?

Jenkins

…enerBus

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.

Jenkins

Author: Shixiong Zhu <shixiong@databricks.com>

Closes #15220 from zsxwing/SPARK-17649.
val droppedEvents = droppedEventsCounter.get
if (droppedEvents > 0) {
// Don't log too frequently
if (System.currentTimeMillis() - lastReportTimestamp >= 60 * 1000) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use nanotime

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't nanotime be overkill ? Even if there is a single dropped event, this check will get executed with every post() so having currentTimeMillis (which is less costly) is preferable.

Copy link
Member Author

@zsxwing zsxwing Sep 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rxin this is not for measuring accurate elapsed time and we also want to log it using Date. According to the javadoc:

This method can only be used to measure elapsed time and is not related to any other notion of system or wall-clock time. The value returned represents nanoseconds since some fixed but arbitrary origin time (perhaps in the future, so values may be negative). The same origin is used by all invocations of this method in an instance of a Java virtual machine; other virtual machine instances are likely to use a different origin.

It's better to use System.currentTimeMillis() since we want to report the timestamp in the log.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

@SparkQA
Copy link

SparkQA commented Sep 24, 2016

Test build #65855 has finished for PR 15226 at commit 0e014b0.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@zsxwing zsxwing changed the title [SPARK-17649][CORE] Log how many Spark events got dropped in AsynchronousListenerBus [SPARK-17649][CORE] Log how many Spark events got dropped in AsynchronousListenerBus (branch 1.6) Sep 26, 2016
@zsxwing
Copy link
Member Author

zsxwing commented Sep 26, 2016

Merging to 1.6.

@zsxwing zsxwing closed this Sep 26, 2016
asfgit pushed a commit that referenced this pull request Sep 26, 2016
…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 <shixiong@databricks.com>

Closes #15226 from zsxwing/SPARK-17649-branch-1.6.
@zsxwing zsxwing deleted the SPARK-17649-branch-1.6 branch September 26, 2016 18:05
zzcclp pushed a commit to zzcclp/spark that referenced this pull request Sep 27, 2016
…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 <shixiong@databricks.com>

Closes apache#15226 from zsxwing/SPARK-17649-branch-1.6.

(cherry picked from commit 7aded55)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants