[SPARK-37727][SQL] Show ignored confs & hide warnings for conf already set in SparkSession.builder.getOrCreate #35001

HyukjinKwon · 2021-12-23T12:39:29Z

What changes were proposed in this pull request?

This PR proposes to show ignored configurations and hide the warnings for configurations that are already set when invoking SparkSession.builder.getOrCreate.

Why are the changes needed?

Currently, SparkSession.builder.getOrCreate() is too noisy even when duplicate configurations are set. Users cannot easily tell which configurations are to fix. See the example below:

./bin/spark-shell --conf spark.abc=abc

import org.apache.spark.sql.SparkSession
spark.sparkContext.setLogLevel("DEBUG")
SparkSession.builder.config("spark.abc", "abc").getOrCreate

21:04:01.670 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing SparkSession; some spark core configurations may not take effect.

It is straitforward when there are few configurations but it is difficult for users to figure out when there are too many configurations especially when these configurations are defined in a property file like 'spark-default.conf' maintained separately by system admins in production.

Does this PR introduce any user-facing change?

Yes.

Show ignored configurations in debug level logs:

./bin/spark-shell --conf spark.abc=abc

import org.apache.spark.sql.SparkSession
spark.sparkContext.setLogLevel("DEBUG")
SparkSession.builder
  .config("spark.sql.warehouse.dir", "2")
  .config("spark.abc", "abcb")
  .config("spark.abcd", "abcb4")
  .getOrCreate

Before:

21:13:28.360 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing SparkSession; the static sql configurations will not take effect.
21:13:28.360 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing SparkSession; some spark core configurations may not take effect.

After:

20:34:30.619 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing Spark session; only runtime SQL configurations will take effect.
20:34:30.622 [main] DEBUG org.apache.spark.sql.SparkSession - Ignored static SQL configurations:
  spark.sql.warehouse.dir=2
20:34:30.623 [main] DEBUG org.apache.spark.sql.SparkSession - Configurations that might not take effect:
  spark.abcd=abcb4
  spark.abc=abcb

Do not issue a warning and hide a configuration already explicitly set (with the same value) before.

./bin/spark-shell --conf spark.abc=abc

import org.apache.spark.sql.SparkSession
spark.sparkContext.setLogLevel("DEBUG")
SparkSession.builder.config("spark.abc", "abc").getOrCreate  // **Ignore** warnings because it's already set in --conf
SparkSession.builder.config("spark.abc.new", "abc").getOrCreate  // **Show** warnings for only configuration newly set.
SparkSession.builder.config("spark.abc.new", "abc").getOrCreate  // **Ignore** warnings because it's set ^.

Before:

21:13:56.183 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing SparkSession; some spark core configurations may not take effect.
21:13:56.356 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing SparkSession; some spark core configurations may not take effect.
21:13:56.476 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing SparkSession; some spark core configurations may not take effect.

After:

20:36:36.251 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing Spark session; only runtime SQL configurations will take effect.
20:36:36.253 [main] DEBUG org.apache.spark.sql.SparkSession - Configurations that might not take effect:
  spark.abc.new=abc

Do not issue a warning and hide runtime SQL configurations in debug log:

./bin/spark-shell

import org.apache.spark.sql.SparkSession
spark.sparkContext.setLogLevel("DEBUG")
SparkSession.builder.config("spark.sql.ansi.enabled", "true").getOrCreate  // **Ignore** warnings for runtime SQL configurations
SparkSession.builder.config("spark.buffer.size", "1234").getOrCreate  // **Show** warnings for Spark core configuration
SparkSession.builder.config("spark.sql.source.specific", "abc").getOrCreate  // **Show** warnings for custom runtime options
SparkSession.builder.config("spark.sql.warehouse.dir", "xyz").getOrCreate  // **Show** warnings for static SQL configurations

Before:

11:11:40.846 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing SparkSession; some spark core configurations may not take effect.
11:11:41.037 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing SparkSession; some spark core configurations may not take effect.
11:11:41.167 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing SparkSession; some spark core configurations may not take effect.
11:11:41.318 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing SparkSession; the static sql configurations will not take effect.

After:

10:39:54.870 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing Spark session; only runtime SQL configurations will take effect.
10:39:54.872 [main] DEBUG org.apache.spark.sql.SparkSession - Configurations that might not take effect:
  spark.buffer.size=1234
10:39:54.988 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing Spark session; only runtime SQL configurations will take effect.
10:39:54.988 [main] DEBUG org.apache.spark.sql.SparkSession - Configurations that might not take effect:
  spark.sql.source.specific=abc
10:39:55.107 [main] WARN  org.apache.spark.sql.SparkSession - Using an existing Spark session; only runtime SQL configurations will take effect.
10:39:55.108 [main] DEBUG org.apache.spark.sql.SparkSession - Ignored static SQL configurations:
  spark.sql.warehouse.dir=xyz

Note that there is no behaviour change on session state initialization when configurations are not set. For example:

import org.apache.spark.sql.SparkSession
spark.sparkContext.setLogLevel("DEBUG")
SparkSession.builder.getOrCreate

But the session state initialization can be triggered now for static SQL configurations set after this PR. Previously, it was not triggered. This would not introduce something user-facing or a bug but worth noting it.
For runtime SQL configurations, the session state initialization in this code path was introduced at #15295.

How was this patch tested?

It was manually tested as shown above.

HyukjinKwon · 2021-12-23T12:40:00Z

cc @cloud-fan, @dongjoon-hyun @AngersZhuuuu @yaooqinn @maropu FYI when you find some time to review :-).

SparkQA · 2021-12-23T13:48:11Z

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

SparkQA · 2021-12-23T14:59:46Z

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

yaooqinn

LGTM

Ngone51 · 2021-12-23T15:20:54Z

20:36:36.253 [main] DEBUG org.apache.spark.sql.SparkSession - Configurations that might not take effect:
spark.abc.new=abc

I feel this could be a bit confused. Users may thought the value abc doesn't take effect for the conf spark.abc.new. However, it does. The thing is, it's actually the second set doesn't take effect here rather than the value itself.

dongjoon-hyun

+1, LGTM. Thank you, @HyukjinKwon .

SparkQA · 2021-12-23T17:47:01Z

Test build #146522 has finished for PR 35001 at commit f58f6c3.

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

SparkQA · 2021-12-23T17:58:44Z

Test build #146527 has finished for PR 35001 at commit f710487.

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

HyukjinKwon · 2021-12-24T00:01:22Z

I feel this could be a bit confused. Users may thought the value abc doesn't take effect for the conf spark.abc.new. However, it does. The thing is, it's actually the second set doesn't take effect here rather than the value itself.

This actually was already there. The problem here is that users, for example, might pass some source specific options that Spark side does not now. Maybe we can filter out Spark's runtime configuration here though. Let me take a look while I am here.

…y set in SparkSession.builder.getOrCreate

HyukjinKwon · 2021-12-24T01:42:13Z

@Ngone51, the current change should address your comment (see 3. in Pr desscription)

AngersZhuuuu · 2021-12-24T02:07:22Z

Maybe we can have a UT in SparkShellSuite?

HyukjinKwon · 2021-12-24T02:10:45Z

Maybe we can have a UT in SparkShellSuite?

There is actually a bug I guess from switching to log4j 2 (see SPARK-37729). I would prefer to add it after this gets resolved.

AngersZhuuuu · 2021-12-24T02:32:53Z

Maybe we can have a UT in SparkShellSuite?

There is actually a bug I guess from switching to log4j 2 (see SPARK-37729). I would prefer to add it after this gets resolved.

Got it, thanks.

AngersZhuuuu

LGTM

SparkQA · 2021-12-24T02:44:54Z

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

SparkQA · 2021-12-24T03:38:59Z

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

HyukjinKwon · 2021-12-24T03:51:32Z

Merged to master.

HyukjinKwon · 2021-12-24T03:51:41Z

Thanks guys for your reviews.

Ngone51 · 2021-12-24T03:58:49Z

late lgtm

SparkQA · 2021-12-24T06:51:43Z

Test build #146543 has finished for PR 35001 at commit ae4616c.

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

…Session.builder.getOrCreate ### What changes were proposed in this pull request? This PR adds some test cases for #35001. Test cases were not added because there are related bugs but they are fixed now. ### Why are the changes needed? To avoid regressions. ### Does this PR introduce _any_ user-facing change? No, dev-only. ### How was this patch tested? This PR adds test cases that were manually tested in #35001. Closes #35048 from HyukjinKwon/SPARK-37727-followup. Authored-by: Hyukjin Kwon <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>

…Session.builder.getOrCreate ### What changes were proposed in this pull request? This PR adds some test cases for apache#35001. Test cases were not added because there are related bugs but they are fixed now. ### Why are the changes needed? To avoid regressions. ### Does this PR introduce _any_ user-facing change? No, dev-only. ### How was this patch tested? This PR adds test cases that were manually tested in apache#35001. Closes apache#35048 from HyukjinKwon/SPARK-37727-followup. Authored-by: Hyukjin Kwon <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>

github-actions bot added the SQL label Dec 23, 2021

HyukjinKwon mentioned this pull request Dec 23, 2021

[SPARK-37504][PYTHON] Pyspark create SparkSession with existed session should not pass static conf #34757

Closed

HyukjinKwon force-pushed the SPARK-37727 branch 2 times, most recently from f58f6c3 to f710487 Compare December 23, 2021 12:52

yaooqinn approved these changes Dec 23, 2021

View reviewed changes

dongjoon-hyun approved these changes Dec 23, 2021

View reviewed changes

HyukjinKwon added 2 commits December 24, 2021 09:14

[SPARK-37727][SQL] Show ignored confs & hide warnings for conf alread…

24b4859

…y set in SparkSession.builder.getOrCreate

Address a comment

ae4616c

HyukjinKwon force-pushed the SPARK-37727 branch from f710487 to ae4616c Compare December 24, 2021 01:41

AngersZhuuuu approved these changes Dec 24, 2021

View reviewed changes

HyukjinKwon closed this in 472629a Dec 24, 2021

HyukjinKwon mentioned this pull request Dec 29, 2021

[SPARK-37727][SQL][TESTS][FOLLOW-UP] Add test cases for logs in SparkSession.builder.getOrCreate #35048

Closed

HyukjinKwon deleted the SPARK-37727 branch January 4, 2022 00:51

[SPARK-37727][SQL] Show ignored confs & hide warnings for conf already set in SparkSession.builder.getOrCreate #35001

[SPARK-37727][SQL] Show ignored confs & hide warnings for conf already set in SparkSession.builder.getOrCreate #35001

Uh oh!

Conversation

HyukjinKwon commented Dec 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

HyukjinKwon commented Dec 23, 2021

Uh oh!

SparkQA commented Dec 23, 2021

Uh oh!

SparkQA commented Dec 23, 2021

Uh oh!

yaooqinn left a comment

Choose a reason for hiding this comment

Uh oh!

Ngone51 commented Dec 23, 2021

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Dec 23, 2021

Uh oh!

SparkQA commented Dec 23, 2021

Uh oh!

HyukjinKwon commented Dec 24, 2021

Uh oh!

HyukjinKwon commented Dec 24, 2021

Uh oh!

AngersZhuuuu commented Dec 24, 2021

Uh oh!

HyukjinKwon commented Dec 24, 2021

Uh oh!

AngersZhuuuu commented Dec 24, 2021

Uh oh!

AngersZhuuuu left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Dec 24, 2021

Uh oh!

SparkQA commented Dec 24, 2021

Uh oh!

HyukjinKwon commented Dec 24, 2021

Uh oh!

HyukjinKwon commented Dec 24, 2021

Uh oh!

Ngone51 commented Dec 24, 2021

Uh oh!

SparkQA commented Dec 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

HyukjinKwon commented Dec 23, 2021 •

edited

Loading