[SPARK-26700][Core][FOLLOWUP] Add config `spark.network.maxRemoteBlockSizeFetchToMem` by xuanyuanking · Pull Request #27463 · apache/spark

xuanyuanking · 2020-02-05T03:33:02Z

What changes were proposed in this pull request?

Add new config spark.network.maxRemoteBlockSizeFetchToMem fallback to the old config spark.maxRemoteBlockSizeFetchToMem.

Why are the changes needed?

For naming consistency.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests.

cloud-fan · 2020-02-05T04:56:05Z

core/src/main/scala/org/apache/spark/internal/config/package.scala

shall we move it to SparkConf.deprecatedConfigs?

Thansk, done in 2d2637b.

SparkQA · 2020-02-05T06:59:46Z

Test build #117880 has finished for PR 27463 at commit 4201a93.

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

SparkQA · 2020-02-05T07:16:40Z

Test build #117886 has finished for PR 27463 at commit 1939c3f.

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

SparkQA · 2020-02-05T07:49:12Z

Test build #117890 has finished for PR 27463 at commit 4201a93.

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

SparkQA · 2020-02-05T08:05:02Z

Test build #117888 has finished for PR 27463 at commit 77714a3.

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

Ngone51

Do we consider use configsWithAlternatives for this case? Which seems more convenient for such case.

xuanyuanking · 2020-02-05T16:01:37Z

Make senes, done in 5c1f201

SparkQA · 2020-02-05T16:14:16Z

Test build #117932 has finished for PR 27463 at commit 2d2637b.

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

SparkQA · 2020-02-05T19:00:41Z

Test build #117940 has finished for PR 27463 at commit 5c1f201.

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

dongjoon-hyun · 2020-02-05T19:06:55Z

Retest this please.

SparkQA · 2020-02-05T22:01:22Z

Test build #117946 has finished for PR 27463 at commit 5c1f201.

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

cloud-fan · 2020-02-06T05:28:21Z

core/src/main/scala/org/apache/spark/internal/config/package.scala


-  private[spark] val MAX_REMOTE_BLOCK_SIZE_FETCH_TO_MEM =
-    ConfigBuilder("spark.maxRemoteBlockSizeFetchToMem")
+  private[spark] val NETWORK_MAX_REMOTE_BLOCK_SIZE_FETCH_TO_MEM =


we can keep using MAX_REMOTE_BLOCK_SIZE_FETCH_TO_MEM

Thanks, done in 8f7540e.

cloud-fan · 2020-02-06T05:29:34Z

core/src/main/scala/org/apache/spark/SparkConf.scala

-      AlternateConfig("spark.reducer.maxReqSizeShuffleToMem", "2.3")),
+    NETWORK_MAX_REMOTE_BLOCK_SIZE_FETCH_TO_MEM.key -> Seq(
+      AlternateConfig("spark.reducer.maxReqSizeShuffleToMem", "2.3"),
+      AlternateConfig("spark.maxRemoteBlockSizeFetchToMem", "3.0")),


will spark log warning message when these 2 alternatives are used? If not we still need to put them in deprecatedConfigs

It will when user use old one, since deprecation warning also includes configsWithAlternatives:

spark/core/src/main/scala/org/apache/spark/SparkConf.scala

Lines 753 to 766 in 77510c6

def logDeprecationWarning(key: String): Unit = {

deprecatedConfigs.get(key).foreach { cfg =>

logWarning(

s"The configuration key '$key' has been deprecated as of Spark ${cfg.version} and " +

s"may be removed in the future. ${cfg.deprecationMessage}")

return

}

allAlternatives.get(key).foreach { case (newKey, cfg) =>

logWarning(

s"The configuration key '$key' has been deprecated as of Spark ${cfg.version} and " +

s"may be removed in the future. Please use the new key '$newKey' instead.")

return

}

I tested locally, Spark will log warning messages.

SparkQA · 2020-02-06T08:05:02Z

Test build #117973 has finished for PR 27463 at commit 8f7540e.

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

cloud-fan · 2020-02-06T08:14:43Z

retest this please

SparkQA · 2020-02-06T08:28:48Z

Test build #117981 has finished for PR 27463 at commit 8f7540e.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-02-06T08:55:55Z

retest this please

SparkQA · 2020-02-06T09:04:48Z

Test build #117984 has finished for PR 27463 at commit 8f7540e.

This patch fails build dependency tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-02-06T10:06:53Z

retest this please

SparkQA · 2020-02-06T12:36:03Z

Test build #117986 has finished for PR 27463 at commit 8f7540e.

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

cloud-fan · 2020-02-06T12:53:59Z

thanks, merging to master/3.0!

…kSizeFetchToMem` ### What changes were proposed in this pull request? Add new config `spark.network.maxRemoteBlockSizeFetchToMem` fallback to the old config `spark.maxRemoteBlockSizeFetchToMem`. ### Why are the changes needed? For naming consistency. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Existing tests. Closes #27463 from xuanyuanking/SPARK-26700-follow. Authored-by: Yuanjian Li <xyliyuanjian@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit d861357) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

…kSizeFetchToMem` ### What changes were proposed in this pull request? Add new config `spark.network.maxRemoteBlockSizeFetchToMem` fallback to the old config `spark.maxRemoteBlockSizeFetchToMem`. ### Why are the changes needed? For naming consistency. ### Does this PR introduce any user-facing change? No. ### How was this patch tested? Existing tests. Closes apache#27463 from xuanyuanking/SPARK-26700-follow. Authored-by: Yuanjian Li <xyliyuanjian@gmail.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

xuanyuanking force-pushed the SPARK-26700-follow branch from 77714a3 to 4201a93 Compare February 5, 2020 04:21

cloud-fan reviewed Feb 5, 2020

View reviewed changes

Ngone51 reviewed Feb 5, 2020

View reviewed changes

fix

5c1f201

xuanyuanking force-pushed the SPARK-26700-follow branch from 2d2637b to 5c1f201 Compare February 5, 2020 16:00

dongjoon-hyun added the SPARK CORE label Feb 5, 2020

cloud-fan reviewed Feb 6, 2020

View reviewed changes

rename

8f7540e

cloud-fan closed this in d861357 Feb 6, 2020

	def logDeprecationWarning(key: String): Unit = {
	deprecatedConfigs.get(key).foreach { cfg =>
	logWarning(
	s"The configuration key '$key' has been deprecated as of Spark ${cfg.version} and " +
	s"may be removed in the future. ${cfg.deprecationMessage}")
	return
	}

	allAlternatives.get(key).foreach { case (newKey, cfg) =>
	logWarning(
	s"The configuration key '$key' has been deprecated as of Spark ${cfg.version} and " +
	s"may be removed in the future. Please use the new key '$newKey' instead.")
	return
	}

Conversation

xuanyuanking commented Feb 5, 2020

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!

cloud-fan Feb 5, 2020

Choose a reason for hiding this comment

Uh oh!

xuanyuanking Feb 5, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Feb 5, 2020

Uh oh!

SparkQA commented Feb 5, 2020

Uh oh!

SparkQA commented Feb 5, 2020

Uh oh!

SparkQA commented Feb 5, 2020

Uh oh!

Ngone51 left a comment

Choose a reason for hiding this comment

Uh oh!

xuanyuanking commented Feb 5, 2020

Uh oh!

SparkQA commented Feb 5, 2020

Uh oh!

SparkQA commented Feb 5, 2020

Uh oh!

dongjoon-hyun commented Feb 5, 2020

Uh oh!

SparkQA commented Feb 5, 2020

Uh oh!

cloud-fan Feb 6, 2020

Choose a reason for hiding this comment

Uh oh!

xuanyuanking Feb 6, 2020

Choose a reason for hiding this comment

Uh oh!

cloud-fan Feb 6, 2020

Choose a reason for hiding this comment

Uh oh!

Ngone51 Feb 6, 2020 • 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!

xuanyuanking Feb 6, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Feb 6, 2020

Uh oh!

cloud-fan commented Feb 6, 2020

Uh oh!

SparkQA commented Feb 6, 2020

Uh oh!

cloud-fan commented Feb 6, 2020

Uh oh!

SparkQA commented Feb 6, 2020

Uh oh!

cloud-fan commented Feb 6, 2020

Uh oh!

SparkQA commented Feb 6, 2020

Uh oh!

cloud-fan commented Feb 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

Ngone51 Feb 6, 2020 •

edited

Loading