Skip to content

Conversation

@carsonwang
Copy link
Contributor

@carsonwang carsonwang commented Feb 21, 2017

What changes were proposed in this pull request?

In SQLListener.getExecutionMetrics, driver accumulator updates don't belong to the execution should be ignored when merging all accumulator updates to prevent NoSuchElementException.

How was this patch tested?

Updated unit test.

@carsonwang
Copy link
Contributor Author

cc @cloud-fan @zsxwing

@carsonwang carsonwang changed the title [SPARK-19674][SQL]Ignore non-existing driver accumulator updates in SQLListener [SPARK-19674][SQL]Ignore non-existing driver accumulator updates when merging all accumulator updates in SQLListener Feb 21, 2017
@SparkQA
Copy link

SparkQA commented Feb 21, 2017

Test build #73199 has finished for PR 17009 at commit f24bf52.

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


checkAnswer(listener.getExecutionMetrics(0), accumulatorUpdates.mapValues(_ * 2))

// Non-existing driver accumulator updates should be filtered and no exception will be thrown.
Copy link
Contributor

Choose a reason for hiding this comment

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

when will this happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was doing some own experiments that adds physical operators at runtime. The metrics of the added operators are not registered to the execution so I got a NoSuchElementException.

@cloud-fan
Copy link
Contributor

cloud-fan commented Feb 23, 2017

The change looks reasonable, but is it non-existing driver accumulators, or driver accumulators don't belong to this execution?

@carsonwang
Copy link
Contributor Author

Thanks @cloud-fan . driver accumulators don't belong to this execution is more appropriate. I'll update the words.

@carsonwang carsonwang changed the title [SPARK-19674][SQL]Ignore non-existing driver accumulator updates when merging all accumulator updates in SQLListener [SPARK-19674][SQL]Ignore driver accumulator updates don't belong to the execution when merging all accumulator updates Feb 23, 2017
@SparkQA
Copy link

SparkQA commented Feb 23, 2017

Test build #73333 has finished for PR 17009 at commit 62398ec.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Feb 23, 2017

Test build #73360 has finished for PR 17009 at commit 62398ec.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@asfgit asfgit closed this in eff7b40 Feb 23, 2017
Yunni pushed a commit to Yunni/spark that referenced this pull request Feb 27, 2017
…the execution when merging all accumulator updates

## What changes were proposed in this pull request?
In SQLListener.getExecutionMetrics, driver accumulator updates don't belong to the execution should be ignored when merging all accumulator updates to prevent NoSuchElementException.

## How was this patch tested?
Updated unit test.

Author: Carson Wang <[email protected]>

Closes apache#17009 from carsonwang/FixSQLMetrics.
@mallman
Copy link
Contributor

mallman commented Mar 24, 2017

It looks like this will fix a bug we're experiencing in Spark 2.1. Given that this PR is a bug fix, any chance we can get a backport into branch-2.1? I can work on it myself if @carsonwang is unable.

mallman pushed a commit to VideoAmp/spark-public that referenced this pull request Mar 24, 2017
…the execution when merging all accumulator updates

## What changes were proposed in this pull request?
In SQLListener.getExecutionMetrics, driver accumulator updates don't belong to the execution should be ignored when merging all accumulator updates to prevent NoSuchElementException.

## How was this patch tested?
Updated unit test.

Author: Carson Wang <[email protected]>

Closes apache#17009 from carsonwang/FixSQLMetrics.
@mallman
Copy link
Contributor

mallman commented Mar 24, 2017

I created a Spark 2.1 backport at #17418.

asfgit pushed a commit that referenced this pull request Mar 25, 2017
[SPARK-19674][SQL] Ignore driver accumulator updates don't belong to the execution when merging all accumulator updates

N.B. This is a backport to branch-2.1 of #17009.

## What changes were proposed in this pull request?
In SQLListener.getExecutionMetrics, driver accumulator updates don't belong to the execution should be ignored when merging all accumulator updates to prevent NoSuchElementException.

## How was this patch tested?
Updated unit test.

Author: Carson Wang <carson.wangintel.com>

Author: Carson Wang <[email protected]>

Closes #17418 from mallman/spark-19674-backport_2.1.
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