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

Configurably omit offers from hosts that are overloaded #1776

Merged
merged 5 commits into from
Apr 11, 2018

Conversation

ssalinas
Copy link
Member

@ssalinas ssalinas commented Apr 4, 2018

If, according to the metrics collected from the slave metrics endpoint, a slave is overloaded, omit these offers from scoring and do not schedule tasks there. This feature is configurable via omitOverloadedHosts under the mesos config, and defaults to false/off

Overloaded is currently defined as a load5 > 1.0 to catch ongoing load, or a load1 > 1.5 to catch shorter spikes large enough to be disruptive to tasks on the host.

@JsonIgnore
public boolean isOverloaded() {
// Any host where load5 is > 1 is overloaded. Also consider a higher threshold for load1 to take into account spikes large enough to be disruptive
return systemLoad5Min > 1.0 || systemLoad1Min > 1.5;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to make these values tunable knobs as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah was going back and forth on that. Might as well for flexibility. I'll update 👍

@baconmania
Copy link
Contributor

🚢

@baconmania
Copy link
Contributor

🚢

@@ -353,6 +355,11 @@ private double score(SingularityOfferHolder offerHolder, Map<String, Integer> ta
return 0;
}

if (mesosConfiguration.isOmitOverloadedHosts() && maybeSlaveUsage.isPresent() && maybeSlaveUsage.get().isOverloaded()) {
LOG.debug("Slave {} is overloaded (), ignoring offer");
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we missed the interp'd parameters here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in future PR already, since this is already 🚢 ed

@ssalinas ssalinas added this to the 0.20.0 milestone Apr 11, 2018
@ssalinas ssalinas merged commit 05f5f59 into master Apr 11, 2018
@ssalinas ssalinas deleted the omit_overloaded branch April 11, 2018 13:59
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