Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-45845][SS][UI] Add number of evicted state rows to streaming UI #43723

Closed
wants to merge 5 commits into from

Conversation

WweiL
Copy link
Contributor

@WweiL WweiL commented Nov 8, 2023

What changes were proposed in this pull request?

In a stateful query, a watermark has two responsibilities:

  1. drop late rows
  2. Evict state rows from state store

Before we only log "aggregated number of rows dropped by watermark". This is case 1. But people would confuse this with case 2. This PR purpose we also add a chart for case 2. Also made the explanation for case 1 more verbose.

Now:
image

Why are the changes needed?

UI improvement

Does this PR introduce any user-facing change?

Yes, users will be able to view the new UI

How was this patch tested?

Manually tested

Was this patch authored or co-authored using generative AI tooling?

No

@WweiL
Copy link
Contributor Author

WweiL commented Nov 8, 2023

cc @HeartSaVioR can you take a look please : )

<tr>
<td style="vertical-align: middle;">
<div style="width: 160px;">
<div><strong>Aggregated Number Of Evicted State Rows{SparkUIUtils.tooltip("Aggregated number of state rows evicted from the state.", "right")}</strong></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the word "removed" than "evicted" - it is "also" fit to the case where you manually remove the state in flatMapGroupsWithState. The semantic of "eviction" does not cover such case.

Copy link
Contributor

@HeartSaVioR HeartSaVioR left a comment

Choose a reason for hiding this comment

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

Only a minor. Could you please retrigger CI btw? I know it's not failing with your change but let's give a try once more.

@HeartSaVioR
Copy link
Contributor

https://github.com/WweiL/oss-spark/actions/runs/6829179497/job/18579693008

Could you please check with UISeleniumSuite and confirm that it's not related to this change?

@WweiL
Copy link
Contributor Author

WweiL commented Nov 13, 2023

@HeartSaVioR Thanks for pointing it out! Yes the two UISeleniumSuite related ones are indeed caused by this PR. I verified locally that they don't fail after my new commit

Copy link
Contributor

@HeartSaVioR HeartSaVioR left a comment

Choose a reason for hiding this comment

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

+1

@HeartSaVioR
Copy link
Contributor

Thanks! 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.

2 participants