YARN-6673 Add cpu cgroup configurations for opportunistic containers #232
Conversation
There was a problem hiding this comment.
Can we rename CPU_DEFAULT_WEIGHT to CPU_DEFAULT_WEIGHT_GUARANTEED for clarity?
There was a problem hiding this comment.
We can have a small method for reuse I think
boolean isOpportunistic(Container container) {
ContainerTokenIdentifier id = container.getContainerTokenIdentifier();
return id !=null && id.getExecutionType() == ExecutionType.OPPORTUNISTIC;
}
There was a problem hiding this comment.
Hmm this is just 2 lines of code. Adding a function would be at least 5 more lines. Ideally the container has this function but that might be too much churn for the interface.
There was a problem hiding this comment.
yeah. In some sense, it is to be more descriptive than code reuse. Maybe we could have an inline boolean variable with such name?
There was a problem hiding this comment.
If strict resource usage mode is turned on, do we need to do differently for OPPORTUNISTIC containers? Now it bases the cfs_ configurations on vcore
There was a problem hiding this comment.
They still have a CPU setting, right? Or is it 0?
There was a problem hiding this comment.
They are currently based on vcore
There was a problem hiding this comment.
That is a maximum setting and it still needs to be set I think. Why would opportunistic containers have more resource than guaranteed one?
There was a problem hiding this comment.
Not quite get your question. What I meant initially was to ask you if you think base cfs-quota on vcore is the right thing to do.
There was a problem hiding this comment.
Yes, I think so. If the admin chooses strict cpu limits, all containers should get strict cpu limits based on vcores. Opportunistic ones still will be throttled by cpu.shares, if guaranteed are running. This is just a cap, for opportunistic containers with different thread counts not to affect each other negatively.
There was a problem hiding this comment.
I see. Thanks for the clarification.
There was a problem hiding this comment.
Can we move this above cGroupsCpuResourceHandler.prestart()?
… LocalityManager Author: Jacob Maes <[email protected]> Reviewers: Chris Pettitt <[email protected]> Closes apache#232 from jmakes/samza-1346
…Builder reports an issue with the container-executor (apache#7290) (apache#232) Change-Id: Iaaa94c8f46faa4feaede27de36e0d94483ae0229
…Builder reports an issue with the container-executor (apache#7290) (apache#232) (apache#243) Change-Id: Iaaa94c8f46faa4feaede27de36e0d94483ae0229 (cherry picked from commit e6ad8c4)
2 participants
YARN-6673 Add cpu cgroup configurations for opportunistic containers