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

Add prioritization of conflicting constraints #293

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Varsius
Copy link
Contributor

@Varsius Varsius commented Oct 30, 2024

No description provided.

check(
"low=20%, high=80%, crit=95%, step=20%, min=1100",
"size=1000, usage=500, smax=900",
"low->800", "low->900", // StrictMaximumSize takes precedence over minimum constraint
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand why percentage-step resizing goes to 800 here, but I feel weird about StrictMaximumSize = 900 causing a downsize to 800 here. If the downsize goes through, we will upsize back to 900 immediately afterwards (right?). So I would rather have it go to 900 in both cases.

Suggested change
"low->800", "low->900", // StrictMaximumSize takes precedence over minimum constraint
"low->900", "low->900", // StrictMaximumSize takes precedence over minimum constraint

Having said that, if this turns out to be too difficult to do in this PR without making invasive changes, I'm fine with putting this down as a TODO for later. There are more things to do in the constraint engine soon anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this particular case, there wouldn't be an immediate upsize afterwards, since the new usage would be 62,5% which is within all the thresholds.

check( "low=20%, high=80%, crit=95%, step=20%, min=1100", "size=1000, usage=800, smax=900", "low->900", "low->900", // StrictMaximumSize takes precedence over minimum constraint )
In the case that the forced downsize operation would cross the high threshold, the percentage step strategy will also downsize to 900.

Comment on lines 371 to 375
check(
"low=20%, high=80%, crit=95%, step=20%, max=1100",
"size=1000, usage=990, smax=1050",
"critical->1050", "critical->1050", // StrictMaximumSize takes precedence over maximum constraint
)
Copy link
Contributor

Choose a reason for hiding this comment

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

For what it's worth, this test does not establish that "StrictMaximumSize takes precedence over maximum constraint". In fact, two maximum constraints cannot really take precedence over each other. If there is no lower bound, the strongest upper bound will always win. So this testcase would have the same result if the values for max and smax were swapped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree.

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