Skip to content

Conversation

@HeartSaVioR
Copy link
Contributor

What changes were proposed in this pull request?

This PR fixes the representation to clarify the meaning of "number of rows dropped by watermark" in SS UI page.

Why are the changes needed?

Aggregated Number Of State Rows Dropped By Watermark says that the dropped rows are from the state, whereas they're not. We say "evicted from the state" for the case, which is "normal" to emit outputs and reduce memory usage of the state.

The metric actually represents the number of "input" rows dropped by watermark, and the meaning of "input" is relative to the "stateful operator". That's a bit confusing as we normally think "input" as "input from source" whereas it's not.

Does this PR introduce any user-facing change?

Yes, UI element & tooltip change.

How was this patch tested?

Only text change in UI, so we know how thing will be changed intuitively.

@HeartSaVioR
Copy link
Contributor Author

cc. @zsxwing @xuanyuanking @gaborgsomogyi as reviewers & author of #30151

@SparkQA
Copy link

SparkQA commented Nov 20, 2020

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

@SparkQA
Copy link

SparkQA commented Nov 20, 2020

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

@SparkQA
Copy link

SparkQA commented Nov 20, 2020

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

@SparkQA
Copy link

SparkQA commented Nov 20, 2020

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

@SparkQA
Copy link

SparkQA commented Nov 20, 2020

Test build #131396 has finished for PR 30439 at commit ec1fba1.

  • This patch passes all tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Nov 20, 2020

Test build #131397 has finished for PR 30439 at commit d700231.

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

@HeartSaVioR
Copy link
Contributor Author

retest this, please

Copy link
Contributor

@gaborgsomogyi gaborgsomogyi left a comment

Choose a reason for hiding this comment

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

LGTM. Yeah, that's the accurate meaning, thanks!

@SparkQA
Copy link

SparkQA commented Nov 20, 2020

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

@SparkQA
Copy link

SparkQA commented Nov 20, 2020

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

@SparkQA
Copy link

SparkQA commented Nov 20, 2020

Test build #131408 has finished for PR 30439 at commit d700231.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thank you, @HeartSaVioR and @gaborgsomogyi .

+1, LGTM. For me, this PR is ready for merging for Apache Spark 3.1.0.

Do you have something more, @HeartSaVioR ?

@dongjoon-hyun
Copy link
Member

I'll leave this PR to you, @HeartSaVioR .

@HeartSaVioR
Copy link
Contributor Author

HeartSaVioR commented Nov 21, 2020

Thanks all for reviewing. Given this change is just a textural change and the metric was added by me, I don't feel I can get further review comments. Merging to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants