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

Kill tasks below a certain priority level #1052

Merged
merged 24 commits into from
Sep 1, 2016
Merged

Kill tasks below a certain priority level #1052

merged 24 commits into from
Sep 1, 2016

Conversation

tpetr
Copy link
Contributor

@tpetr tpetr commented May 19, 2016

No description provided.

@ssalinas ssalinas modified the milestone: 0.8.0 May 20, 2016
@tpetr tpetr changed the title WIP: priority kill Kill tasks below a certain priority level May 20, 2016
requestIdToTaskPriority.put(requestWithState.getRequest().getId(), requestWithState.getRequest().getTaskPriorityLevel().or(defaultTaskPriorityLevel));
}

// cancel pending tasks below minimum priority level
Copy link
Member

@ssalinas ssalinas Jun 8, 2016

Choose a reason for hiding this comment

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

I'd be worried about deleting pending tasks, because there won't necessarily be a pending request in the queue to put them back when we undo the freeze. We would have to do something like we do on startup where we loop over all tasks. Maybe the mesos scheduler that processes offers should just be smart enough to ignore those pending tasks? (and maybe sort the scheduling by request priority as well?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a fair point -- I certainly don't want to introduce new edge-cases due to deleting pending tasks. I'll update this to ignore pending tasks below the minimum priority level and deal with the fact that it'll cause task lag alerts (which could be good anyways -- a nice reminder that you have a priority freeze enabled...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed your feedback in the latest commit. Not going to tackle sorting scheduled tasks by priority quite yet, but may do it in a future PR.

@tpetr tpetr added the hs_qa label Jun 8, 2016
@ssalinas ssalinas modified the milestone: 0.8.0 Jun 16, 2016
Sort task requests by priority order
@ssalinas ssalinas modified the milestone: 0.10.0 Jul 29, 2016
@ssalinas ssalinas modified the milestones: 0.10.0, 0.11.0 Aug 19, 2016
@ssalinas
Copy link
Member

ssalinas commented Sep 1, 2016

Fixed merge conflict with master, going to merge this one.

@ssalinas ssalinas merged commit 1ae6552 into master Sep 1, 2016
@ssalinas ssalinas deleted the priority-level branch September 1, 2016 14:36
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.

2 participants