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

Constraint numeric comparison #4729

Closed
onlyjob opened this issue Sep 26, 2018 · 8 comments · Fixed by #14722
Closed

Constraint numeric comparison #4729

onlyjob opened this issue Sep 26, 2018 · 8 comments · Fixed by #14722

Comments

@onlyjob
Copy link
Contributor

onlyjob commented Sep 26, 2018

For some reason job with

      constraint {
        attribute = "${attr.cpu.numcores}"
        operator  = ">"
        value     = "10"
      }

Is placed to machines with cpu.numcores == 8.

Why? Even if constraints are not mandatory but merely advisory a job should have been placed to client with 10+ cores, right?

@dadgar dadgar changed the title 0.8.5: constraint violation? Constraint numeric comparison Sep 26, 2018
@dadgar
Copy link
Contributor

dadgar commented Sep 26, 2018

I changed the title of the issue. Constraints are compared lexicographically. We should parse it as a number if possible however. Marking as an enhancement

@onlyjob
Copy link
Contributor Author

onlyjob commented Sep 27, 2018

Thanks, but according to your comment it looks like a bug...

MrJacek pushed a commit to MrJacek/nomad that referenced this issue Nov 8, 2018
@onlyjob
Copy link
Contributor Author

onlyjob commented Nov 7, 2019

Is there any hope to get it fixed?

@tgross
Copy link
Member

tgross commented Nov 7, 2019

It looks like @MrJacek has a patch here MrJacek@74e43fe that was never submitted as a PR. I wonder if we could get him to submit that?

@MrJacek
Copy link

MrJacek commented Nov 7, 2019

It was submitted as a PR in 4856 but rejected :)

@tgross
Copy link
Member

tgross commented Nov 7, 2019

Ah, I see that now. It does look like this should be supported following #4783 where attributes get parsed into numbers, but I just verified it's still lexicographically sorting the constraints. I'll dig into this because the behavior seems like a gotcha even if it's documented.

@tgross tgross added this to Needs Triage in Nomad - Community Issues Triage via automation Nov 7, 2019
@tgross tgross self-assigned this Nov 7, 2019
@tgross tgross moved this from Needs Triage to Triaged in Nomad - Community Issues Triage Nov 7, 2019
@onlyjob
Copy link
Contributor Author

onlyjob commented Nov 7, 2019

Thanks, @tgross. I agree that documenting this bug is not enough. It should be fixed as current behaviour is illogical, counter-intuitive and potent to cause very undesirable placement issues.

It does not seem to be a difficult technical issue to fix, right?

@tgross tgross added this to the near-term milestone Nov 15, 2019
@tgross tgross modified the milestones: near-term, unscheduled Nov 20, 2019
@tgross tgross assigned tgross and unassigned tgross Nov 20, 2019
@tgross tgross removed this from Triaged in Nomad - Community Issues Triage Nov 20, 2019
@tgross tgross added this to Needs Roadmapping in Nomad - Community Issues Triage Feb 12, 2021
@tgross tgross removed this from the unscheduled milestone Feb 12, 2021
@tgross tgross removed this from Needs Roadmapping in Nomad - Community Issues Triage Mar 4, 2021
shoenig added a commit that referenced this issue Sep 27, 2022
This PR changes constraint comparisons to be numeric rather than
lexical if both operands are integers or floats.

Inspiration #4856
Closes #4729
Closes #14719
shoenig added a commit that referenced this issue Sep 27, 2022
* cleanup: fixup linter warnings in schedular/feasible.go

* core: numeric operands comparisons in constraints

This PR changes constraint comparisons to be numeric rather than
lexical if both operands are integers or floats.

Inspiration #4856
Closes #4729
Closes #14719

* fix: always parse as int64
@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, 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 Jan 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants