[SPARK-24637][SS] Add metrics regarding state and watermark to dropwizard metrics #21622

HeartSaVioR · 2018-06-23T12:41:48Z

What changes were proposed in this pull request?

The patch adds metrics regarding state and watermark to dropwizard metrics, so that watermark and state rows/size can be tracked via time-series manner.

How was this patch tested?

Manually tested with CSV metric sink.

…etrics

HeartSaVioR · 2018-06-23T12:42:44Z

cc. @tdas @zsxwing @jose-torres @jerryshao @arunmahadevan @HyukjinKwon

HeartSaVioR · 2018-06-23T13:06:12Z

I think we may want to add metrics regarding sources and sinks as well, but the format of offset information or other metadata information can be different between sources and sinks.
Not sure about more preferred approach: 1. define general format of information for source/sink 2. let individual source/sink manage metric as well.

HyukjinKwon · 2018-06-23T14:47:18Z

add to whitelist

SparkQA · 2018-06-23T15:42:16Z

Test build #92256 has finished for PR 21622 at commit 147c98a.

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

HeartSaVioR · 2018-06-23T15:52:30Z

retest this, please

SparkQA · 2018-06-23T16:34:46Z

Test build #92260 has finished for PR 21622 at commit 147c98a.

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

SparkQA · 2018-06-23T19:32:40Z

Test build #92261 has finished for PR 21622 at commit 147c98a.

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

arunmahadevan · 2018-06-26T18:15:12Z

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

+  timestampFormat.setTimeZone(DateTimeUtils.getTimeZone("UTC"))
+
+  registerGauge("eventTime-watermark",
+    s => convertStringDateToMillis(s.eventTime.get("watermark")), 0L)


nit: rename s => progress to make it clear.

The eventTime-watermark metrics needs to be reported only if the map is not empty (event time). Could be skipped if the map is empty (processing time) to avoid confusion.

will address

We don't know whether the map will be empty when calling registerGauge, and once we register the metric, getValue in Gauge is called from Dropwizard so I'm not sure we can control whether reporting the value or not.

arunmahadevan · 2018-06-26T18:16:37Z

Looks good overall, a couple of minor comments.

SparkQA · 2018-06-27T00:58:03Z

Test build #92355 has finished for PR 21622 at commit 722e6a0.

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

HeartSaVioR · 2018-07-06T05:51:59Z

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

+
+  registerGauge("states-rowsTotal", _.stateOperators.map(_.numRowsTotal).sum, 0L)
+  registerGauge("states-usedBytes", _.stateOperators.map(_.memoryUsedBytes).sum, 0L)
+


We can add more metrics like "providerLoadedMapSizeBytes" after adopting SPARK-24441, so that actual memory usage of state store provider could be tracked via time-series manner.

Those are custom metrics, which may or may not be present depending on the implementation of state store. I dont recommend adding them here directly.

Thanks for the input! I'll keep the patch as it is.

Could you suggest some approaches to extend the maintained metrics? I would like to expand more, and newer things might be coming from custom metrics (like from source and sink) so might be worth to have extension point.

My question is beyond of this PR, so please continue reviewing the patch. Thanks!

jose-torres · 2018-07-10T16:24:11Z

lgtm

HeartSaVioR · 2018-07-10T20:37:58Z

Thanks for reviewing @arunmahadevan and @jose-torres ! Could we finalize review on #21469 to see a chance to include "providerLoadedMapSizeBytes" to here? Or is it OK to handle it with follow-up issue?

HeartSaVioR · 2018-08-01T05:30:44Z

Pinging @tdas and @zsxwing for reviewing. It's small one.

HyukjinKwon · 2018-08-01T07:09:42Z

retest this please

SparkQA · 2018-08-01T10:04:55Z

Test build #93874 has finished for PR 21622 at commit 722e6a0.

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

HeartSaVioR · 2018-08-01T21:37:24Z

Test failure looks unrelated.

Jenkins, retest this, please

HyukjinKwon · 2018-08-02T03:59:58Z

retest this please

SparkQA · 2018-08-02T07:05:01Z

Test build #93941 has finished for PR 21622 at commit 722e6a0.

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

HeartSaVioR · 2018-08-06T01:37:54Z

retest this please

SparkQA · 2018-08-06T05:05:55Z

Test build #94261 has finished for PR 21622 at commit 722e6a0.

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

HeartSaVioR · 2018-08-06T22:55:54Z

@HyukjinKwon Could you take this forward given that the patch is minor and CI test is passed? Thanks in advance!

HyukjinKwon · 2018-08-07T02:11:59Z

Merged to master.

HeartSaVioR · 2018-08-07T02:34:07Z

Thanks @HyukjinKwon for merging, and thanks all for reviewing!

SPARK-24637 Add metrics regarding state and watermark to dropwizard m…

147c98a

…etrics

arunmahadevan reviewed Jun 26, 2018

View reviewed changes

Address minor review comment

722e6a0

HeartSaVioR mentioned this pull request Jul 6, 2018

[SPARK-24748][SS] Support for reporting custom metrics via StreamingQuery Progress #21721

Closed

HeartSaVioR commented Jul 6, 2018

View reviewed changes

asfgit closed this in 6afe6f3 Aug 7, 2018

HeartSaVioR deleted the SPARK-24637 branch January 25, 2019 22:22


		registerGauge("states-rowsTotal", _.stateOperators.map(_.numRowsTotal).sum, 0L)
		registerGauge("states-usedBytes", _.stateOperators.map(_.memoryUsedBytes).sum, 0L)

[SPARK-24637][SS] Add metrics regarding state and watermark to dropwizard metrics #21622

[SPARK-24637][SS] Add metrics regarding state and watermark to dropwizard metrics #21622

Uh oh!

Conversation

HeartSaVioR commented Jun 23, 2018

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

HeartSaVioR commented Jun 23, 2018

Uh oh!

HeartSaVioR commented Jun 23, 2018

Uh oh!

HyukjinKwon commented Jun 23, 2018

Uh oh!

SparkQA commented Jun 23, 2018

Uh oh!

HeartSaVioR commented Jun 23, 2018

Uh oh!

SparkQA commented Jun 23, 2018

Uh oh!

SparkQA commented Jun 23, 2018

Uh oh!

arunmahadevan Jun 26, 2018

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR Jun 26, 2018

Choose a reason for hiding this comment

Uh oh!

arunmahadevan commented Jun 26, 2018

Uh oh!

SparkQA commented Jun 27, 2018

Uh oh!

HeartSaVioR Jul 6, 2018

Choose a reason for hiding this comment

Uh oh!

tdas Aug 1, 2018

Choose a reason for hiding this comment

Uh oh!

HeartSaVioR Aug 1, 2018 • 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!

jose-torres commented Jul 10, 2018

Uh oh!

HeartSaVioR commented Jul 10, 2018

Uh oh!

HeartSaVioR commented Aug 1, 2018

Uh oh!

HyukjinKwon commented Aug 1, 2018

Uh oh!

SparkQA commented Aug 1, 2018

Uh oh!

HeartSaVioR commented Aug 1, 2018

Uh oh!

HyukjinKwon commented Aug 2, 2018

Uh oh!

SparkQA commented Aug 2, 2018

Uh oh!

HeartSaVioR commented Aug 6, 2018

Uh oh!

SparkQA commented Aug 6, 2018

Uh oh!

HeartSaVioR commented Aug 6, 2018

Uh oh!

HyukjinKwon commented Aug 7, 2018

Uh oh!

HeartSaVioR commented Aug 7, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

HeartSaVioR Aug 1, 2018 •

edited

Loading