Skip to content

[SPARK-11082][YARN] Fix wrong core number when response vcore is less than requested vcore #9095

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.

Sign up for GitHub

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

Closed
jerryshao wants to merge 1 commit into from
Closed

[SPARK-11082][YARN] Fix wrong core number when response vcore is less than requested vcore #9095

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,10 @@ private[yarn] class YarnAllocator(
val executorId = executorIdCounter.toString

assert(container.getResource.getMemory >= resource.getMemory)
Copy link
Contributor Author

@jerryshao jerryshao Oct 13, 2015

Choose a reason for hiding this comment

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

@srowen , we already had such defensive code for memory.

Copy link
Member

@srowen srowen Oct 13, 2015

Choose a reason for hiding this comment

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

True, I mean, should we expect there is a case where the granted memory is less than requested as well? and allow or handle it? right now it's rejected, so I expect it can't happen. But then again the code seemed to assume that (sort of) about vcores too.

Copy link
Contributor Author

@jerryshao jerryshao Oct 13, 2015

Choose a reason for hiding this comment

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

Not sure from Yarn side the granted memory will possibly be less than the requested, I haven't met such problem yet.

Copy link
Contributor

@tgravescs tgravescs Oct 13, 2015

Choose a reason for hiding this comment

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

memory should never be less then requested. vcores support was added later though and if its configured off or the scheduler doesn't support it then its possible to get back less. Like mentioned the defaultResourceCalculator just always returns 1.

There is already a comment on this at matchContainerToRequest. Is this actually failing or you were just surprised at what you got?

Copy link
Contributor Author

@jerryshao jerryshao Oct 13, 2015

Choose a reason for hiding this comment

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

It will not fail, just made me quite confused when looking at the cores I set is different from what displayed in yarn side.

Copy link

@jdesmet jdesmet Apr 30, 2016

Choose a reason for hiding this comment

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

Note that wrong vcore accounting in Yarn can affect system integrity due to over-scheduling the CPU. It is mandatory to have this working correctly if spark has to play a nice citizen on yarn (together with other scheduled apps or itself).

if (container.getResource.getVirtualCores < resource.getVirtualCores) {
logWarning(s"Number of vcore ${container.getResource.getVirtualCores} in allocated " +
s"container response is less than the requested number ${resource.getVirtualCores}")
}

logInfo("Launching container %s for on host %s".format(containerId, executorHostname))
executorIdToContainer(executorId) = container
Expand All @@ -414,7 +418,7 @@ private[yarn] class YarnAllocator(
executorId,
executorHostname,
executorMemory,
executorCores,
container.getResource.getVirtualCores,
Copy link
Contributor

@tgravescs tgravescs Oct 13, 2015

Choose a reason for hiding this comment

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

does this actually fail when using executorCores?

appAttemptId.getApplicationId.toString,
securityMgr)
if (launchContainers) {
Expand Down