[SPARK-30479][SQL] Apply compaction of event log to SQL events #27164

HeartSaVioR · 2020-01-10T07:38:19Z

What changes were proposed in this pull request?

This patch addresses adding event filter to handle SQL related events. This patch is next task of SPARK-29779 (#27085), please refer the description of PR #27085 to see overall rationalization of this patch.

Below functionalities will be addressed in later parts:

integrate compaction into FsHistoryProvider
documentation about new configuration

Why are the changes needed?

One of major goal of SPARK-28594 is to prevent the event logs to become too huge, and SPARK-29779 achieves the goal. We've got another approach in prior, but the old approach required models in both KVStore and live entities to guarantee compatibility, while they're not designed to do so.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Added UTs.

HeartSaVioR · 2020-01-10T07:39:14Z

Will rebase once SPARK-29779 is merged. For now, the only effective commit is the last, c72fd1f.

SparkQA · 2020-01-10T08:05:02Z

Test build #116461 has finished for PR 27164 at commit c72fd1f.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

HeartSaVioR · 2020-01-11T05:03:41Z

OK Just rebased this. cc. @vanzin @gaborgsomogyi

SparkQA · 2020-01-11T06:51:47Z

Test build #116524 has finished for PR 27164 at commit 5f37b64.

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

dongjoon-hyun · 2020-01-12T03:44:11Z

Retest this please.

SparkQA · 2020-01-12T05:47:25Z

Test build #116543 has finished for PR 27164 at commit 5f37b64.

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

HeartSaVioR · 2020-01-13T02:57:56Z

retest this, please

SparkQA · 2020-01-13T05:27:44Z

Test build #116585 has finished for PR 27164 at commit 5f37b64.

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

HeartSaVioR · 2020-01-13T05:59:55Z

retest this, please

SparkQA · 2020-01-13T07:54:33Z

Test build #116605 has finished for PR 27164 at commit 5f37b64.

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

HeartSaVioR · 2020-01-13T12:50:32Z

retest this, please

SparkQA · 2020-01-13T15:13:52Z

Test build #116637 has finished for PR 27164 at commit 5f37b64.

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

HeartSaVioR · 2020-01-13T23:05:34Z

pyspark.mllib.tests.test_streaming_algorithms.StreamingLinearRegressionWithTests.test_train_prediction
pyspark.mllib.tests.test_streaming_algorithms.StreamingLogisticRegressionWithSGDTests.test_parameter_accuracy
pyspark.mllib.tests.test_streaming_algorithms.StreamingLogisticRegressionWithSGDTests.test_training_and_prediction

Not relevant failures.

HeartSaVioR · 2020-01-13T23:05:41Z

retest this, please

SparkQA · 2020-01-14T01:33:03Z

Test build #116665 has finished for PR 27164 at commit 5f37b64.

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

SparkQA · 2020-01-14T06:43:49Z

Test build #116680 has finished for PR 27164 at commit 01ece29.

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

vanzin · 2020-01-15T00:13:10Z

Looks ok to me. Will leave here until tomorrow to see if anyone comments.

vanzin · 2020-01-15T18:47:04Z

Merging to master.

HeartSaVioR · 2020-01-15T21:12:23Z

Thanks for reviewing and merging!

…Server ### What changes were proposed in this pull request? This patch addresses remaining functionality on event log compaction: integrate compaction into FsHistoryProvider. This patch is next task of SPARK-30479 (#27164), please refer the description of PR #27085 to see overall rationalization of this patch. ### Why are the changes needed? One of major goal of SPARK-28594 is to prevent the event logs to become too huge, and SPARK-29779 achieves the goal. We've got another approach in prior, but the old approach required models in both KVStore and live entities to guarantee compatibility, while they're not designed to do so. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Added UT. Closes #27208 from HeartSaVioR/SPARK-30481. Authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan.opensource@gmail.com> Signed-off-by: Marcelo Vanzin <vanzin@apache.org>

HyukjinKwon · 2020-01-31T03:19:56Z

sql/core/src/main/scala/org/apache/spark/sql/execution/history/SQLEventFilterBuilder.scala

+  private val _stageToRDDs = new mutable.HashMap[Int, Set[Int]]
+  private val stages = new mutable.HashSet[Int]
+
+  def liveSQLExecutions: Set[Long] = _liveExecutionToJobs.keySet.toSet


quick question: is this non-private only because of the testing purpose?

Yes. Maybe we could reduce down the scope to package private.

HyukjinKwon · 2020-01-31T03:22:06Z

sql/core/src/main/scala/org/apache/spark/sql/execution/history/SQLEventFilterBuilder.scala

+ * between finished job and live job without relation of SQL execution.
+ */
+private[spark] class SQLEventFilterBuilder extends SparkListener with EventFilterBuilder {
+  private val _liveExecutionToJobs = new mutable.HashMap[Long, mutable.Set[Int]]


another nit or quick question. Why did you use leading underscore? Seems the name doesn't conflict.

That was conflicted with below methods, and I renamed the methods as the purpose of methods changed slightly. Revisiting this now, looks like these fields don't need to have underscore.

HyukjinKwon · 2020-01-31T03:22:56Z

sql/core/src/main/scala/org/apache/spark/sql/execution/history/SQLEventFilterBuilder.scala

+    _liveJobs: Set[Int],
+    _liveStages: Set[Int],
+    _liveTasks: Set[Long],
+    _liveRDDs: Set[Int])


Here too. The naming here seems confusing ...

They're just temporal variables and the only purpose is to pass to the constructor of JobEventFilter. As they don't conflict with parameters on the constructor of JobEventFilter, I wouldn't mind removing underscore.

HeartSaVioR · 2020-01-31T07:17:02Z

Thanks for the post-hoc review. I'm seeing review comments from all of three PRs - let me deal with all of reviews at once.

HyukjinKwon · 2020-01-31T07:17:49Z

Thanks @HeartSaVioR!

…ts on post-hoc review ### What changes were proposed in this pull request? This PR reflects review comments on post-hoc review among PRs for SPARK-29779 (#27085), SPARK-30479 (#27164). The list of review comments this PR addresses are below: * #27085 (comment) * #27164 (comment) * #27164 (comment) * #27164 (comment) I also applied review comments to the CORE module (BasicEventFilterBuilder.scala) as well, as the review comments for SQL/core module (SQLEventFilterBuilder.scala) can be applied there as well. ### Why are the changes needed? There're post-hoc reviews on PRs for such issues, like links in above section. ### Does this PR introduce any user-facing change? No ### How was this patch tested? Existing UTs. Closes #27414 from HeartSaVioR/SPARK-28869-SPARK-29779-SPARK-30479-FOLLOWUP-posthoc-reviews. Authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan.opensource@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

HeartSaVioR mentioned this pull request Jan 10, 2020

[WIP][SPARK-29779][SQL] Compact old event log files and cleanup - part 2 #27086

Closed

[SPARK-30479][SQL] Apply compaction of event log to SQL events

5f37b64

HeartSaVioR force-pushed the SPARK-30479 branch from c72fd1f to 5f37b64 Compare January 11, 2020 05:01

HeartSaVioR changed the title ~~[WIP][SPARK-30479][SQL] Apply compaction of event log to SQL events~~ [SPARK-30479][SQL] Apply compaction of event log to SQL events Jan 11, 2020

dongjoon-hyun added the SQL label Jan 12, 2020

Refine comment a bit

01ece29

HeartSaVioR mentioned this pull request Jan 15, 2020

[SPARK-30481][CORE] Integrate event log compactor into Spark History Server #27208

Closed

vanzin closed this in e751bc6 Jan 15, 2020

HeartSaVioR deleted the SPARK-30479 branch January 15, 2020 21:12

HyukjinKwon reviewed Jan 31, 2020

View reviewed changes

HeartSaVioR mentioned this pull request Jan 31, 2020

[SPARK-29779][SPARK-30479][CORE][SQL][FOLLOWUP] Reflect review comments on post-hoc review #27414

Closed

[SPARK-30479][SQL] Apply compaction of event log to SQL events #27164

[SPARK-30479][SQL] Apply compaction of event log to SQL events #27164

Uh oh!

Conversation

HeartSaVioR commented Jan 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

HeartSaVioR commented Jan 10, 2020

Uh oh!

SparkQA commented Jan 10, 2020

Uh oh!

HeartSaVioR commented Jan 11, 2020

Uh oh!

SparkQA commented Jan 11, 2020

Uh oh!

dongjoon-hyun commented Jan 12, 2020

Uh oh!

SparkQA commented Jan 12, 2020

Uh oh!

HeartSaVioR commented Jan 13, 2020

Uh oh!

SparkQA commented Jan 13, 2020

Uh oh!

HeartSaVioR commented Jan 13, 2020

Uh oh!

SparkQA commented Jan 13, 2020

Uh oh!

HeartSaVioR commented Jan 13, 2020

Uh oh!

SparkQA commented Jan 13, 2020

Uh oh!

HeartSaVioR commented Jan 13, 2020

Uh oh!

HeartSaVioR commented Jan 13, 2020

Uh oh!

SparkQA commented Jan 14, 2020

Uh oh!

SparkQA commented Jan 14, 2020

Uh oh!

vanzin commented Jan 15, 2020

Uh oh!

vanzin commented Jan 15, 2020

Uh oh!

HeartSaVioR commented Jan 15, 2020

Uh oh!

HyukjinKwon Jan 31, 2020

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR Jan 31, 2020

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Jan 31, 2020

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR Jan 31, 2020

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Jan 31, 2020

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR Jan 31, 2020

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR commented Jan 31, 2020

Uh oh!

HyukjinKwon commented Jan 31, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

HeartSaVioR commented Jan 10, 2020 •

edited

Loading