[SPARK-35788][SS] Metrics support for RocksDB instance #32934

xuanyuanking · 2021-06-16T15:01:52Z

What changes were proposed in this pull request?

Add more metrics for the RocksDB instance. We transform the native states from RocksDB.

Why are the changes needed?

Improve the usability with more metrics for RocksDB instance.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests.

SparkQA · 2021-06-16T15:47:59Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44408/

SparkQA · 2021-06-16T16:20:53Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44408/

SparkQA · 2021-06-16T16:58:56Z

Test build #139878 has finished for PR 32934 at commit f8f9d20.

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

viirya · 2021-06-23T06:50:17Z

retest this please

viirya

I don't find the different change than other related PRs. @xuanyuanking Which is different change here?

xuanyuanking · 2021-06-23T08:05:25Z

Sorry @viirya, my mistake. Just push the latest commit of the metrics support.

SparkQA · 2021-06-23T08:18:26Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44717/

SparkQA · 2021-06-23T08:27:33Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44717/

SparkQA · 2021-06-23T09:23:11Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44722/

SparkQA · 2021-06-23T09:41:12Z

Test build #140190 has finished for PR 32934 at commit f8f9d20.

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

SparkQA · 2021-06-23T11:58:23Z

Test build #140195 has finished for PR 32934 at commit db2a2b7.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class RocksDBMetrics(
case class RocksDBNativeHistogram(
case class RocksDBFileManagerMetrics(

SparkQA · 2021-06-29T18:39:13Z

Test build #140372 has finished for PR 32934 at commit 92e014e.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-06-30T14:46:31Z

Test build #140447 has finished for PR 32934 at commit 1dfe279.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-06-30T14:48:31Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44961/

SparkQA · 2021-06-30T15:44:21Z

Test build #140451 has finished for PR 32934 at commit 522bac9.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-06-30T16:44:51Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44965/

HeartSaVioR · 2021-07-02T08:53:45Z

Shall we rebase this one for continue reviewing?

xuanyuanking · 2021-07-02T09:00:11Z

Yea, just finished the rebasing.

SparkQA · 2021-07-02T09:08:37Z

Test build #140571 has finished for PR 32934 at commit eea3aa0.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

HeartSaVioR · 2021-07-02T09:14:17Z

Same rebasing issue happens here as well.

Could we please do the "clean" rebase? Like below:

git checkout master
git pull
git checkout -b SPARK-35788-v2
git cherry-pick 7b6da3d (and following commits)
git log <= note the latest commit ID (say, "commit_id")
git checkout SPARK-35788
git reset --hard "commit_id" (referring `SPARK-35788-v2` here might work. if then no need to note the commit ID)
git push --force

xuanyuanking · 2021-07-02T09:30:42Z

Thanks for the guidance, let me try.

SparkQA · 2021-07-02T09:52:31Z

Test build #140576 has finished for PR 32934 at commit a580a1d.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class RocksDBMetrics(
case class RocksDBNativeHistogram(
case class RocksDBFileManagerMetrics(

SparkQA · 2021-07-02T09:53:59Z

Kubernetes integration test unable to build dist.

exiting with code: 1
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45087/

SparkQA · 2021-07-02T13:50:27Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45099/

SparkQA · 2021-07-02T14:24:05Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45099/

SparkQA · 2021-07-02T16:40:07Z

Test build #140586 has finished for PR 32934 at commit 1b1adc5.

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

HeartSaVioR

Looks good overall. Only nits.

HeartSaVioR · 2021-07-05T01:05:53Z

sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDBFileManager.scala

+ * Metrics regarding RocksDB file sync between local and DFS.
+ */
+case class RocksDBFileManagerMetrics(
+  filesCopied: Long,


nit: indent

Thanks, done in 355953e.

HeartSaVioR · 2021-07-05T01:22:00Z

sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDBFileManager.scala

+   */
+  @volatile private var saveCheckpointMetrics = RocksDBFileManagerMetrics.EMPTY_METRICS
+
+  def latestloadCheckpointMetrics: RocksDBFileManagerMetrics = loadCheckpointMetrics


nit: latestLoadCheckpointMetrics

Thanks, done in 355953e

HeartSaVioR · 2021-07-05T01:39:10Z

sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDB.scala


+/** Class to represent stats from each commit. */
+case class RocksDBMetrics(
+  numCommittedKeys: Long,


nit: indent

Thanks, done in 355953e

HeartSaVioR · 2021-07-05T01:39:39Z

sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDB.scala

+
+/** Class to wrap RocksDB's native histogram */
+case class RocksDBNativeHistogram(
+  avg: Double, stddev: Double, median: Double, p95: Double, p99: Double) {


nit: indent

Thanks, done in 355953e

SparkQA · 2021-07-05T15:38:25Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45172/

SparkQA · 2021-07-05T16:11:42Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45172/

SparkQA · 2021-07-05T17:33:56Z

Test build #140661 has finished for PR 32934 at commit 355953e.

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

xuanyuanking · 2021-07-06T00:17:02Z

retest this please

HeartSaVioR

+1

I'll wait for @viirya to take a look; I'll merge this in a couple of days if there's no comment.

SparkQA · 2021-07-06T01:41:34Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45183/

viirya · 2021-07-06T02:01:35Z

I'm in vacation, but I will find some time looking at this. Thanks.

HeartSaVioR · 2021-07-06T02:08:40Z

Thanks for the update. Let's just merge this in and add up the reviews in the next PR then. It should be pretty much simpler as we will have only one PR for RocksDB state store after merging this.

HeartSaVioR · 2021-07-06T02:10:26Z

Thanks! Merging to master/3.2.

viirya · 2021-07-06T02:10:54Z

Okay for me. Thanks!

SparkQA · 2021-07-06T02:14:42Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/45183/

### What changes were proposed in this pull request? Add more metrics for the RocksDB instance. We transform the native states from RocksDB. ### Why are the changes needed? Improve the usability with more metrics for RocksDB instance. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing tests. Closes #32934 from xuanyuanking/SPARK-35788. Authored-by: Yuanjian Li <[email protected]> Signed-off-by: Jungtaek Lim <[email protected]> (cherry picked from commit 9544277) Signed-off-by: Jungtaek Lim <[email protected]>

HeartSaVioR · 2021-07-06T02:28:30Z

Thanks @xuanyuanking for the contribution! I merged this to master/branch-3.2.

Please rebase the next PR.

SparkQA · 2021-07-06T05:09:35Z

Test build #140672 has finished for PR 32934 at commit 355953e.

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

github-actions bot added BUILD CORE SQL STRUCTURED STREAMING labels Jun 16, 2021

xuanyuanking mentioned this pull request Jun 16, 2021

[SPARK-35628][SS] RocksDBFileManager - load checkpoint from DFS #32767

Closed

viirya reviewed Jun 23, 2021

View reviewed changes

xuanyuanking force-pushed the SPARK-35788 branch from db2a2b7 to 92e014e Compare June 28, 2021 12:40

xuanyuanking force-pushed the SPARK-35788 branch from 92e014e to 1dfe279 Compare June 30, 2021 14:43

xuanyuanking force-pushed the SPARK-35788 branch from 522bac9 to eea3aa0 Compare July 2, 2021 09:00

xuanyuanking changed the title ~~[WIP][SPARK-35788][SS] Metrics support for RocksDB instance~~ [SPARK-35788][SS] Metrics support for RocksDB instance Jul 2, 2021

more metrics

a580a1d

xuanyuanking force-pushed the SPARK-35788 branch from eea3aa0 to a580a1d Compare July 2, 2021 09:38

fix

1b1adc5

HeartSaVioR reviewed Jul 5, 2021

View reviewed changes

address comments

355953e

HeartSaVioR approved these changes Jul 6, 2021

View reviewed changes

HeartSaVioR closed this in 9544277 Jul 6, 2021

xuanyuanking deleted the SPARK-35788 branch July 6, 2021 02:26

HeartSaVioR mentioned this pull request Jul 6, 2021

[SPARK-35988][SS] The implementation for RocksDBStateStoreProvider #33187

Closed

[SPARK-35788][SS] Metrics support for RocksDB instance #32934

[SPARK-35788][SS] Metrics support for RocksDB instance #32934

Uh oh!

Conversation

xuanyuanking commented Jun 16, 2021

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!

SparkQA commented Jun 16, 2021

Uh oh!

SparkQA commented Jun 16, 2021

Uh oh!

SparkQA commented Jun 16, 2021

Uh oh!

viirya commented Jun 23, 2021

Uh oh!

viirya left a comment

Choose a reason for hiding this comment

Uh oh!

xuanyuanking commented Jun 23, 2021

Uh oh!

SparkQA commented Jun 23, 2021

Uh oh!

SparkQA commented Jun 23, 2021

Uh oh!

SparkQA commented Jun 23, 2021

Uh oh!

SparkQA commented Jun 23, 2021

Uh oh!

SparkQA commented Jun 23, 2021

Uh oh!

SparkQA commented Jun 29, 2021

Uh oh!

SparkQA commented Jun 30, 2021

Uh oh!

SparkQA commented Jun 30, 2021

Uh oh!

SparkQA commented Jun 30, 2021

Uh oh!

SparkQA commented Jun 30, 2021

Uh oh!

HeartSaVioR commented Jul 2, 2021

Uh oh!

xuanyuanking commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

HeartSaVioR commented Jul 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

xuanyuanking commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

SparkQA commented Jul 2, 2021

Uh oh!

HeartSaVioR left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR commented Jul 2, 2021 •

edited

Loading

HeartSaVioR left a comment •

edited

Loading

HeartSaVioR commented Jul 6, 2021 •

edited

Loading