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-29543][SS][FOLLOWUP] Move spark.sql.streaming.ui.* configs to StaticSQLConf #27425

Closed
wants to merge 2 commits into from

Conversation

xuanyuanking
Copy link
Member

What changes were proposed in this pull request?

Put the configs below needed by Structured Streaming UI into StaticSQLConf:

  • spark.sql.streaming.ui.enabled
  • spark.sql.streaming.ui.retainedProgressUpdates
  • spark.sql.streaming.ui.retainedQueries

Why are the changes needed?

Make all SS UI configs consistent with other similar configs in usage and naming.

Does this PR introduce any user-facing change?

Yes, add new static config spark.sql.streaming.ui.retainedProgressUpdates.

How was this patch tested?

Existing UT.

@xuanyuanking
Copy link
Member Author

cc @zsxwing @uncleGen

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-29543][SS][Follow Up] The configs in Structured Streaming UI should be put into StaticSQLConf [SPARK-29543][SS][FOLLOWUP] Move spark.sql.streaming.ui.* configs to StaticSQLConf Feb 1, 2020
@SparkQA
Copy link

SparkQA commented Feb 1, 2020

Test build #117709 has finished for PR 27425 at commit e4c3b38.

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

Copy link
Member

@zsxwing zsxwing left a comment

Choose a reason for hiding this comment

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

Thanks for fixing these. Left some nits. Otherwise, LGTM.


val STREAMING_UI_ENABLED =
buildStaticConf("spark.sql.streaming.ui.enabled")
.doc("Whether to run the structured streaming UI for the Spark application.")
Copy link
Member

Choose a reason for hiding this comment

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

nit: Whether to run the Structured Streaming Web UI for the Spark application when the Spark Web UI is enabled.


val STREAMING_UI_RETAINED_PROGRESS_UPDATES =
buildStaticConf("spark.sql.streaming.ui.retainedProgressUpdates")
.doc("The number of progress updates to retain for a streaming query for structured " +
Copy link
Member

Choose a reason for hiding this comment

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

nit: structured streaming ui -> Structured Streaming UI


val STREAMING_UI_RETAINED_QUERIES =
buildStaticConf("spark.sql.streaming.ui.retainedQueries")
.doc("The number of inactive queries to retain for structured streaming ui.")
Copy link
Member

Choose a reason for hiding this comment

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

ditto

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, doc changes done in d763abb.

@@ -146,12 +146,14 @@ private[sql] class SharedState(
*/
lazy val streamingQueryStatusListener: Option[StreamingQueryStatusListener] = {
val sqlConf = SQLConf.get
Copy link
Member

Choose a reason for hiding this comment

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

nit: we can remove this and use conf in this class directly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, then StreamingQueryStatusListener need a little change, done in d763abb.

@SparkQA
Copy link

SparkQA commented Feb 2, 2020

Test build #117740 has finished for PR 27425 at commit d763abb.

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

@uncleGen
Copy link
Contributor

uncleGen commented Feb 3, 2020

Thanks for this work, LGTM.

@zsxwing
Copy link
Member

zsxwing commented Feb 3, 2020

Thanks! Merging to master and branch-3.0.

@asfgit asfgit closed this in a4912ce Feb 3, 2020
asfgit pushed a commit that referenced this pull request Feb 3, 2020
…o StaticSQLConf

### What changes were proposed in this pull request?
Put the configs below needed by Structured Streaming UI into StaticSQLConf:

- spark.sql.streaming.ui.enabled
- spark.sql.streaming.ui.retainedProgressUpdates
- spark.sql.streaming.ui.retainedQueries

### Why are the changes needed?
Make all SS UI configs consistent with other similar configs in usage and naming.

### Does this PR introduce any user-facing change?
Yes, add new static config `spark.sql.streaming.ui.retainedProgressUpdates`.

### How was this patch tested?
Existing UT.

Closes #27425 from xuanyuanking/SPARK-29543-follow.

Authored-by: Yuanjian Li <xyliyuanjian@gmail.com>
Signed-off-by: Shixiong Zhu <zsxwing@gmail.com>
(cherry picked from commit a4912ce)
Signed-off-by: Shixiong Zhu <zsxwing@gmail.com>
@xuanyuanking
Copy link
Member Author

Thanks for the review!

@xuanyuanking xuanyuanking deleted the SPARK-29543-follow branch February 3, 2020 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants