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

Denormalize jobs in plan and ignore resources of terminal allocs #4713

Merged
merged 3 commits into from
Sep 24, 2018

Conversation

dadgar
Copy link
Contributor

@dadgar dadgar commented Sep 22, 2018

Denormalize jobs in AppendAllocs
AppendAlloc was originally only ever
called for inplace upgrades and new allocations. Both these code paths
would remove the job from the allocation. Now we use this to also add
fields such as FollowupEvalID which did not normalize the job. This is only
a performance enhancement.

Ignore terminal allocs
Failed allocations are annotated with the followup
Eval ID when one is created to replace the failed allocation. However, in
the plan applier, when we check if allocations fit, these terminal
allocations were not filtered. This could result in the plan being rejected
if the node would be overcommited if the terminal allocations resources
were considered.

Builds on #4709

@dadgar dadgar changed the title Denormalize jobs in plan and ignore resources of terminal allocs WIP: Denormalize jobs in plan and ignore resources of terminal allocs Sep 22, 2018
@dadgar dadgar changed the base branch from master to b-jet-fixes September 24, 2018 20:53
Denormalize jobs in AppendAllocs:
AppendAlloc was originally only ever called for inplace upgrades and new
allocations. Both these code paths would remove the job from the
allocation. Now we use this to also add fields such as FollowupEvalID
which did not normalize the job. This is only a performance enhancement.

Ignore terminal allocs:
Failed allocations are annotated with the followup Eval ID when one is
created to replace the failed allocation. However, in the plan applier,
when we check if allocations fit, these terminal allocations were not
filtered. This could result in the plan being rejected if the node would
be overcommited if the terminal allocations resources were considered.
@dadgar dadgar changed the title WIP: Denormalize jobs in plan and ignore resources of terminal allocs Denormalize jobs in plan and ignore resources of terminal allocs Sep 24, 2018
@schmichael schmichael changed the base branch from b-jet-fixes to b-deployments September 24, 2018 21:19
@@ -7138,6 +7141,10 @@ func (p *Plan) PopUpdate(alloc *Allocation) {
func (p *Plan) AppendAlloc(alloc *Allocation) {
Copy link
Member

Choose a reason for hiding this comment

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

Add comment

@dadgar dadgar merged commit 13da821 into b-deployments Sep 24, 2018
@dadgar dadgar deleted the b-plan branch September 24, 2018 23:05
@dadgar dadgar restored the b-plan branch September 24, 2018 23:06
@dadgar dadgar deleted the b-plan branch September 24, 2018 23:07
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 27, 2023
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.

2 participants