Skip to content

Comments

[SPARK-31923][Core]Ignore internal accumulators that use unrecognized types rather than crashing (branch-2.4)#28758

Closed
zsxwing wants to merge 2 commits intoapache:branch-2.4from
zsxwing:SPARK-31923-2.4
Closed

[SPARK-31923][Core]Ignore internal accumulators that use unrecognized types rather than crashing (branch-2.4)#28758
zsxwing wants to merge 2 commits intoapache:branch-2.4from
zsxwing:SPARK-31923-2.4

Conversation

@zsxwing
Copy link
Member

@zsxwing zsxwing commented Jun 8, 2020

What changes were proposed in this pull request?

Backport #28744 to branch-2.4.

Why are the changes needed?

Low risky fix for branch-2.4.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

New unit tests.

zsxwing added 2 commits June 8, 2020 13:20
…d types rather than crashing

Ignore internal accumulators that use unrecognized types rather than crashing so that an event log containing such accumulators can still be converted to JSON and logged.

A user may use internal accumulators by adding the `internal.metrics.` prefix to the accumulator name to hide sensitive information from UI (Accumulators except internal ones will be shown in Spark UI).

However, `org.apache.spark.util.JsonProtocol.accumValueToJson` assumes an internal accumulator has only 3 possible types: `int`, `long`, and `java.util.List[(BlockId, BlockStatus)]`. When an internal accumulator uses an unexpected type, it will crash.

An event log that contains such accumulator will be dropped because it cannot be converted to JSON, and it will cause weird UI issue when rendering in Spark History Server. For example, if `SparkListenerTaskEnd` is dropped because of this issue, the user will see the task is still running even if it was finished.

It's better to make `accumValueToJson` more robust because it's up to the user to pick up the accumulator name.

No

The new unit tests.

Closes apache#28744 from zsxwing/fix-internal-accum.

Authored-by: Shixiong Zhu <zsxwing@gmail.com>
Signed-off-by: Shixiong Zhu <zsxwing@gmail.com>
expectedValue = None)
}

test("SPARK-30936: forwards compatibility - ignore unknown fields") {
Copy link
Member Author

Choose a reason for hiding this comment

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

These two tests are not in branch-2.4, which causes the conflicts.

@tdas
Copy link
Contributor

tdas commented Jun 8, 2020

LGTM. assuming tests pass.

@SparkQA
Copy link

SparkQA commented Jun 8, 2020

Test build #123648 has finished for PR 28758 at commit 2e64849.

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

asfgit pushed a commit that referenced this pull request Jun 8, 2020
…d types rather than crashing (branch-2.4)

### What changes were proposed in this pull request?

Backport #28744 to branch-2.4.

### Why are the changes needed?

Low risky fix for branch-2.4.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

New unit tests.

Closes #28758 from zsxwing/SPARK-31923-2.4.

Authored-by: Shixiong Zhu <zsxwing@gmail.com>
Signed-off-by: Shixiong Zhu <zsxwing@gmail.com>
@zsxwing zsxwing closed this Jun 8, 2020
@zsxwing zsxwing deleted the SPARK-31923-2.4 branch June 8, 2020 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants