Skip to content
This repository has been archived by the owner on Jan 9, 2020. It is now read-only.

Support specify CPU cores for driver pod #207

Merged

Conversation

hustcat
Copy link

@hustcat hustcat commented Mar 29, 2017

Support specify CPU cores for driver pod, such as:

--conf spark.driver.cores=2

Fix #205 .

@foxish
Copy link
Member

foxish commented Mar 29, 2017

Thanks for the PR @hustcat. This lgtm. Thoughts @mcheah?

Copy link

@ash211 ash211 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -64,6 +64,9 @@ private[spark] class Client(
.map(_.split(","))
.getOrElse(Array.empty[String])

// CPU settings
private val driverCpuCores = sparkConf.getOption("spark.driver.cores").getOrElse("1")
Copy link

Choose a reason for hiding this comment

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

If there's a config like org.apache.spark.internal.config.DRIVER_MEMORY for driver cores that would be good to use, if not no need to make one

Copy link
Author

@hustcat hustcat Mar 30, 2017

Choose a reason for hiding this comment

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

Yeah, there's not a config like org.apache.spark.internal.config.DRIVER_MEMORY. IMO, we should change the spark core code less as far as possible before we merge to spark master branch, and change kubernetes scheduler will be better.

@mcheah
Copy link

mcheah commented Mar 29, 2017

I am not involved on this project, you have the wrong developer.

@ash211
Copy link

ash211 commented Mar 29, 2017

I think foxish meant @mccheah (note the extra 'c')

@ash211 ash211 merged commit 35a5e32 into apache-spark-on-k8s:branch-2.1-kubernetes Mar 30, 2017
@ash211
Copy link

ash211 commented Mar 30, 2017

Thanks for the contribution @hustcat !

foxish pushed a commit that referenced this pull request Jul 24, 2017
puneetloya pushed a commit to puneetloya/spark that referenced this pull request Mar 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants