Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Sep 30, 2015

@ghost ghost changed the title spark on yarn support priority option [SPARK-10879] spark on yarn support priority option Sep 30, 2015
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@jerryshao
Copy link
Contributor

From my understanding, priority support on Yarn is still on working, at least for capacity scheduler, right? Also do we need to specify priority for each ContainerRequest.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd really prefer to not add this. There's no need, just set the value in SparkConf and read it in Client.scala - which is what this line you're adding in SparkSubmit already does:

OptionAssigner(args.priority, YARN, CLIENT, sysProp = "spark.yarn.priority"),

Just make it work for both deploy modes.

@vanzin
Copy link
Contributor

vanzin commented Sep 30, 2015

Echoing Saisai's comment, if this work is still not completely finished on YARN, it might be better to not expose a command line option in Spark just yet; just make it a non-documented config option.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe they are going to have a default priority per queue setting so if we hardcode the default to 0 that might not mach what the queues default it. I would rather leave this unset if not explicitly specified.

@tgravescs
Copy link
Contributor

the yarn api parts of this are done and the setPriority method for the applicationsubmissioncontext has been there forever (at least yarn 2.2 and I think before). It just never did anything.
So I don't see an issue with adding this now. Priorities are at an application level not a container level.

I personally would either like to see --priority removed and just use the config or at least renamed to like queuePriority. Right now its only used by yarn and you can just as easily set this just by --conf spark.yarn.priority=10 so would rather not see the submit option.

@jerryshao
Copy link
Contributor

Agreed with undocumented configuration.

BTW, my question is that do we need to take care of priority in ContainerRequest? Now it is setting by RM_REQUEST_PRIORITY, if we both specify the application priority and container priority, which one has the priority to overwrite another?

@tgravescs
Copy link
Contributor

Container priority is different. container priority is honored right now and its for the application to say one container must be allocated before another. An example is MapReduce where maps may be higher priority because the reducer can't run until all the maps are finished. At this point I don't see this useful to Spark as all the executors are the same importance.

@xiaowen147 can you update based on the feedback?

@ghost
Copy link
Author

ghost commented Oct 8, 2015

@tgravescs @jerryshao @vanzin Thank you for your suggestions. Due to the YARN's unsupported feature, even if I update, it won't work. So, I consider to close this issue at the moment.
You may refer to https://issues.apache.org/jira/browse/YARN-1963.

@tgravescs
Copy link
Contributor

@xiaowen147 can you clarify, do you just mean its not fully implemented yet?

@vanzin
Copy link
Contributor

vanzin commented Oct 15, 2015

@xiaowen147 did you mean that you won't be updating this PR? Could you close it in that case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants