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-20134][SQL] SQLMetrics.postDriverMetricUpdates to simplify driver side metric updates #17464

Closed
wants to merge 2 commits into from

Conversation

rxin
Copy link
Contributor

@rxin rxin commented Mar 29, 2017

What changes were proposed in this pull request?

It is not super intuitive how to update SQLMetric on the driver side. This patch introduces a new SQLMetrics.postDriverMetricUpdates function to do that, and adds documentation to make it more obvious.

How was this patch tested?

Updated a test case to use this method.

Copy link
Member

@ueshin ueshin left a comment

Choose a reason for hiding this comment

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

LGTM except for a minor comment, pending Jenkins.


SQLMetrics.postDriverMetricUpdates(
sparkContext,
sc.getLocalProperty(SQLExecution.EXECUTION_ID_KEY),
Copy link
Member

Choose a reason for hiding this comment

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

Should these be used the same variable name, sparkContext or sc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm for some reason both works. i agree it'd be better to be the same.

@SparkQA
Copy link

SparkQA commented Mar 29, 2017

Test build #75344 has started for PR 17464 at commit 3886e09.

@SparkQA
Copy link

SparkQA commented Mar 29, 2017

Test build #75343 has finished for PR 17464 at commit 360e9d7.

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

@rxin
Copy link
Contributor Author

rxin commented Mar 29, 2017

Merging in master/branch-2.1.

asfgit pushed a commit that referenced this pull request Mar 29, 2017
…ver side metric updates

## What changes were proposed in this pull request?
It is not super intuitive how to update SQLMetric on the driver side. This patch introduces a new SQLMetrics.postDriverMetricUpdates function to do that, and adds documentation to make it more obvious.

## How was this patch tested?
Updated a test case to use this method.

Author: Reynold Xin <[email protected]>

Closes #17464 from rxin/SPARK-20134.

(cherry picked from commit 9712bd3)
Signed-off-by: Reynold Xin <[email protected]>
@asfgit asfgit closed this in 9712bd3 Mar 29, 2017
@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/75344/
Test FAILed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants