Skip to content

[SPARK-18143][SQL]Ignore Structured Streaming event logs to avoid breaking history server - #15663

Closed
zsxwing wants to merge 5 commits into
apache:masterfrom
zsxwing:fix-event-log
Closed

[SPARK-18143][SQL]Ignore Structured Streaming event logs to avoid breaking history server#15663
zsxwing wants to merge 5 commits into
apache:masterfrom
zsxwing:fix-event-log

Conversation

@zsxwing

@zsxwingzsxwing commented Oct 27, 2016

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

Because of the refactoring work in Structured Streaming, the event logs generated by Strucutred Streaming in Spark 2.0.0 and 2.0.1 cannot be parsed.

This PR just ignores these logs in ReplayListenerBus because no places use them.

How was this patch tested?

  • Generated events logs using Spark 2.0.0 and 2.0.1, and saved them as structured-streaming-query-event-logs-2.0.0.txt and structured-streaming-query-event-logs-2.0.1.txt
  • The new added test makes sure ReplayListenerBus will skip these bad jsons.

@zsxwing

zsxwing commented Oct 27, 2016

Copy link
Copy Markdown
MemberAuthor

cc @marmbrus@tdas

@zsxwing

zsxwing commented Oct 27, 2016

Copy link
Copy Markdown
MemberAuthor

@rxin do we need to cancel 2.0.2-RC1 because of this issue?

tdas
tdas reviewed Oct 27, 2016
View reviewed changes
Comment threadcore/src/main/scala/org/apache/spark/scheduler/ReplayListenerBus.scala Outdated
case e: java.lang.ClassNotFoundException if
e.getMessage == "org.apache.spark.sql.streaming.StreamingQueryListener$QueryProgress" ||
e.getMessage == "org.apache.spark.sql.streaming.StreamingQueryListener$QueryTerminated"
=>

@tdastdasOct 27, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: formatting is weird.
wont it be easier if you defined a set with these class names, and then use "contains"?

@tdas

tdas commented Oct 27, 2016

Copy link
Copy Markdown
Contributor

one comment, but overall looks good.

@SparkQA

SparkQA commented Oct 27, 2016

Copy link
Copy Markdown

Test build #67664 has finished for PR 15663 at commit 5feb1bd.

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

@SparkQA

SparkQA commented Oct 27, 2016

Copy link
Copy Markdown

Test build #67666 has finished for PR 15663 at commit 3aedd53.

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

@SparkQA

SparkQA commented Oct 28, 2016

Copy link
Copy Markdown

Test build #67678 has finished for PR 15663 at commit 4dd84eb.

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

@SparkQA

SparkQA commented Oct 28, 2016

Copy link
Copy Markdown

Test build #67680 has finished for PR 15663 at commit 0f51b4e.

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

tdas
tdas reviewed Oct 28, 2016
View reviewed changes
Comment threadsql/core/src/test/scala/org/apache/spark/sql/streaming/StreamingQueryListenerSuite.scala
@@ -206,6 +209,45 @@ class StreamingQueryListenerSuite extends StreamTest with BeforeAndAfter {
assert(queryQueryTerminated.exception === newQueryTerminated.exception)
}

@tdastdasOct 28, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this test is in sql/core, why are the resources in history server?

@tdastdasOct 28, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I should have noticed this earlier. Sorry!

@tdastdasOct 28, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Duh! its in sql/core :(

But good to create a structured-streaming directory rather than history-server

@zsxwingzsxwingOct 28, 2016

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Renamed

@zsxwing

zsxwing commented Oct 29, 2016

Copy link
Copy Markdown
MemberAuthor

Updated. 2.0.1 event logs don't include the "stackTrace" field.

@SparkQA

SparkQA commented Oct 29, 2016

Copy link
Copy Markdown

Test build #67731 has finished for PR 15663 at commit 729d50d.

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

@SparkQA

SparkQA commented Oct 29, 2016

Copy link
Copy Markdown

Test build #67734 has finished for PR 15663 at commit d30a3a5.

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

@zsxwing

zsxwing commented Oct 29, 2016

Copy link
Copy Markdown
MemberAuthor

retest this please

1 similar comment
@zsxwing

zsxwing commented Oct 29, 2016

Copy link
Copy Markdown
MemberAuthor

retest this please

@SparkQA

SparkQA commented Oct 29, 2016

Copy link
Copy Markdown

Test build #67742 has finished for PR 15663 at commit d30a3a5.

  • This patch fails from timeout after a configured wait of 250m.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA

SparkQA commented Oct 29, 2016

Copy link
Copy Markdown

Test build #67751 has finished for PR 15663 at commit d30a3a5.

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

@rxin

rxin commented Oct 31, 2016

Copy link
Copy Markdown
Contributor

cc @tdas / @marmbrus

does this change lgtu?

@tdas

tdas commented Oct 31, 2016

Copy link
Copy Markdown
Contributor

LGTM. Merging to master and 2.0

@tdas

tdas commented Oct 31, 2016

Copy link
Copy Markdown
Contributor

Okay, I merged only to master, this does not merge cleanly to 2.0. I am on vacation so i cant test out the 2.0 fix quickly, better make a separate PR for 2.0

@zsxwingzsxwing mentioned this pull request Oct 31, 2016
Closed
asfgit pushed a commit that referenced this pull request Oct 31, 2016
@zsxwing@rxin
[SPARK-18143][SQL] Ignore Structured Streaming event logs to avoid br…
300d596
…eaking history server (branch 2.0)
## What changes were proposed in this pull request?
Backport #15663 to branch-2.0 and fixed conflicts in `ReplayListenerBus`.
## How was this patch tested?
Jenkins
Author: Shixiong Zhu <shixiong@databricks.com>
Closes#15695 from zsxwing/fix-event-log-2.0.
robert3005 pushed a commit to palantir/spark that referenced this pull request Nov 1, 2016
@zsxwingRobert Kruszewski
[SPARK-18143][SQL] Ignore Structured Streaming event logs to avoid br…
dab585c
…eaking history server
## What changes were proposed in this pull request?
Because of the refactoring work in Structured Streaming, the event logs generated by Strucutred Streaming in Spark 2.0.0 and 2.0.1 cannot be parsed.
This PR just ignores these logs in ReplayListenerBus because no places use them.
## How was this patch tested?
- Generated events logs using Spark 2.0.0 and 2.0.1, and saved them as `structured-streaming-query-event-logs-2.0.0.txt` and `structured-streaming-query-event-logs-2.0.1.txt`
- The new added test makes sure ReplayListenerBus will skip these bad jsons.
Author: Shixiong Zhu <shixiong@databricks.com>
Closesapache#15663 from zsxwing/fix-event-log.
@zsxwing
zsxwing deleted the fix-event-log branch November 17, 2016 22:31
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
@zsxwing@uzadude
[SPARK-18143][SQL] Ignore Structured Streaming event logs to avoid br…
5e1b273
…eaking history server
## What changes were proposed in this pull request?
Because of the refactoring work in Structured Streaming, the event logs generated by Strucutred Streaming in Spark 2.0.0 and 2.0.1 cannot be parsed.
This PR just ignores these logs in ReplayListenerBus because no places use them.
## How was this patch tested?
- Generated events logs using Spark 2.0.0 and 2.0.1, and saved them as `structured-streaming-query-event-logs-2.0.0.txt` and `structured-streaming-query-event-logs-2.0.1.txt`
- The new added test makes sure ReplayListenerBus will skip these bad jsons.
Author: Shixiong Zhu <shixiong@databricks.com>
Closesapache#15663 from zsxwing/fix-event-log.
@gengliangwanggengliangwang mentioned this pull request Dec 12, 2017
Closed
asfgit pushed a commit that referenced this pull request Dec 13, 2017
@gengliangwang@gatorsmile
[SPARK-22763][CORE] SHS: Ignore unknown events and parse through the …
1abcbed
…file
## What changes were proposed in this pull request?
While spark code changes, there are new events in event log: #19649
And we used to maintain a whitelist to avoid exceptions: #15663
Currently Spark history server will stop parsing on unknown events or unrecognized properties. We may still see part of the UI data.
For better compatibility, we can ignore unknown events and parse through the log file.
## How was this patch tested?
Unit test
Author: Wang Gengliang <ltnwgl@gmail.com>
Closes#19953 from gengliangwang/ReplayListenerBus.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@tdastdastdas left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@zsxwing@tdas@SparkQA@rxin