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

Explain quota change limits #640

Open
Krock21 opened this issue Jun 25, 2024 · 10 comments
Open

Explain quota change limits #640

Krock21 opened this issue Jun 25, 2024 · 10 comments
Labels

Comments

@Krock21
Copy link
Member

Krock21 commented Jun 25, 2024

When trying to change quota on accounts page, I see an error This value must be >= X.
Screenshot 2024-06-25 at 16 40 10

This can be confusing (and was confusing) since there is no explanation on why this limit was enforced. In my case it was enforced by limit from children account and by usage

Also it is hard to understand where the number X comes from. From this code I see some non-trivial logic

I suggest adding (i) icon next to an error with an explanation, like:

  • Hardcode limit
  • Children limits add up to X
  • Usage is X
@Krock21
Copy link
Member Author

Krock21 commented Jan 24, 2025

For some limits this validation is computed as (usage in this node + limits in all children)
I suggest making validation for max(usage in this node + usage in children, limits in children)
And showing warning for values between max(usage in this node + usage in children, limits in children) and (usage in this node + limits in all children)

@ma-efremoff
Copy link
Collaborator

Does your user use @allow_children_limit_overcommit attribute for his accounts?
@Krock21, please provide an example of such configuration to reproduce with ytsaurus/local.

@Krock21
Copy link
Member Author

Krock21 commented Jan 24, 2025

Does your user use @allow_children_limit_overcommit attribute for his accounts?

No

@Krock21, please provide an example of such configuration to reproduce with ytsaurus/local.

  1. Create account structure. Parent with limit = 10, 2 children with limits = 3
Image 2. Fill parent account with 8 nodes Image 3. Try to increase limits for parent account just a little bit (from 10 to 11), but UI requires 14 Image

Issue 1: It is unclear where this 14 comes from. Let's add a hint with explanation

In this case, UI takes usage in this node + limits in all children to calculate validation limit (8 + (3 + 3) = 14)
While correct one would be maximum of usage in this node + usage in children and limits in children. This ensures that configuration is not violating situation on the cluster

Issue 2: Use correct limit for validation, as described above

@ma-efremoff
Copy link
Collaborator

It looks like UI does correct calculations. The issue might be solved by enabling @allow_children_limit_overcommit attribute on parent account.

@Krock21
Copy link
Member Author

Krock21 commented Jan 24, 2025

It looks like UI does correct calculations.

Depends what we mean by "correct". My expectation of correct is different here

  • children_limit_overcommit = %true allows overcommitting children limits over parent limit
  • children_limit_overcommit = %false, on opposite, should require children limits to stay within parent limit

UI enforces staying within parent limit - usage, not limit. And it leads to validation that is higher than current value (which is already something strange)

The issue might be solved by enabling @allow_children_limit_overcommit attribute on parent account.

It has a negative side effect of allowing overcommiting. Imagine same scenario as above, but with usage = 4 in parent.
Image
UI allows setting quota to 4 and 5, which is less than total children limit (3+3=6). This is not what we want here
Image
We want to have a validation of 6, ignoring usage of parent


There is one more example:

Parent with limit = 10 and usage = 5
2 children with limit = 3

Image

Validation for parent limit is >= parent usage + children limits. In this case it is 5+3+3=11 (I disagree with it, I think it should be max(5+0+0,3+3) = 6

Image

BUT, validation for children limit is <= parent limit - other children limits. In this case it is 10-3=7 (I agree with it)

Image

If we consider parent usage, then for children this limit should be parent limit - parent usage - other children limits, which can even go negative. In this case it would be 10 - 5 - 3 = 2

This example show that logic is inconsistent between parent and children limits. There is no "correct" definition that satisfies all

@vrozaev
Copy link
Collaborator

vrozaev commented Feb 7, 2025

Documentation about quota.

@ma-efremoff
Copy link
Collaborator

We still believe the check is useful but we don't mind to allow to disable the check for a specific installation.

@Krock21
Copy link
Member Author

Krock21 commented Feb 25, 2025

Can we make a warning instead?

@Krock21
Copy link
Member Author

Krock21 commented Feb 25, 2025

I suggest the following sequence of checks

  • If new account quota is less than total usage in the account and it's children -- show an error and refuse to change quota
  • If children overcommit is allowed: allow and skip next checks
  • If new account quota is less than sum of children quotas -- show an error and refuse to change quota
  • If new account quota is less than (usage in it + sum of children quotas) -- show a warning and allow to change quota
  • Otherwise, allow

@Krock21
Copy link
Member Author

Krock21 commented Feb 25, 2025

There is also a 2nd part of the issue: explaining where this validation comes from. Users often don't understand that

I suggest adding (i) icon next to an error with an explanation, like:

  • Hardcode limit
  • Children limits add up to X
  • Usage is X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants