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-26466][CORE] Use ConfigEntry for hardcoded configs for submit categories. #23532

Closed
wants to merge 3 commits into from

Conversation

HeartSaVioR
Copy link
Contributor

What changes were proposed in this pull request?

The PR makes hardcoded configs below to use ConfigEntry.

  • spark.kryo
  • spark.kryoserializer
  • spark.serializer
  • spark.jars
  • spark.files
  • spark.submit
  • spark.deploy
  • spark.worker

This patch doesn't change configs which are not relevant to SparkConf (e.g. system properties).

How was this patch tested?

Existing tests.

@SparkQA
Copy link

SparkQA commented Jan 14, 2019

Test build #101151 has finished for PR 23532 at commit e76866b.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HeartSaVioR
Copy link
Contributor Author

Ran compilation in my machine and passed.

mvn -Phadoop-2.7 -Pkubernetes -Pkubernetes-integration-tests -Phive-thriftserver -Pkinesis-asl -Pyarn -Pspark-ganglia-lgpl -Phive -Pmesos clean compile test-compile

@SparkQA
Copy link

SparkQA commented Jan 14, 2019

Test build #101152 has finished for PR 23532 at commit e56b307.

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

spark.conf.set("spark.kryoserializer.buffer.max", "50b")

// this is needed to set configurations which are also defined to SparkConf
spark.conf.set(SET_COMMAND_REJECTS_SPARK_CORE_CONFS.key, "false")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to set this to false because by default it doesn't allow setting configuration which is defined in SparkConf. I'm not fully sure which one is right to do: 1) make the change as I just did 2) don't add spark.kryoserializer.buffer and spark.kryoserializer.buffer.max to Kryo.

@SparkQA
Copy link

SparkQA commented Jan 14, 2019

Test build #101174 has finished for PR 23532 at commit c263104.

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

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

This is looking good @HeartSaVioR ; could you rebase?

Following are addressed:
* spark.kryo
* spark.kryoserializer
* spark.serializer
* spark.jars
* spark.files
* spark.submit
* spark.deploy
* spark.worker
@HeartSaVioR
Copy link
Contributor Author

@srowen Thanks for reviewing! I've just rebased.

@SparkQA
Copy link

SparkQA commented Jan 16, 2019

Test build #101323 has finished for PR 23532 at commit b00669d.

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

@srowen
Copy link
Member

srowen commented Jan 17, 2019

Merged to master

@srowen srowen closed this in 38f0307 Jan 17, 2019
@HeartSaVioR
Copy link
Contributor Author

Thanks for reviewing and merging, @srowen !

@HeartSaVioR HeartSaVioR deleted the SPARK-26466-v2 branch January 25, 2019 22:13
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
…categories.

## What changes were proposed in this pull request?

The PR makes hardcoded configs below to use `ConfigEntry`.

* spark.kryo
* spark.kryoserializer
* spark.serializer
* spark.jars
* spark.files
* spark.submit
* spark.deploy
* spark.worker

This patch doesn't change configs which are not relevant to SparkConf (e.g. system properties).

## How was this patch tested?

Existing tests.

Closes apache#23532 from HeartSaVioR/SPARK-26466-v2.

Authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan@gmail.com>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants