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

[YUNIKORN-2471] Documentation update for max 0 queue resources. #423

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tommyfufu
Copy link

@tommyfufu tommyfufu commented Apr 21, 2024

What is this PR for?

A few sentences describing the overall goals of the pull request's commits.
First time? Check out the contributing guide - http://yunikorn.apache.org/community/how_to_contribute

What type of PR is it?

  • - Bug Fix
  • - Improvement
  • - Feature
  • - Documentation
  • - Hot Fix
  • - Refactoring

Todos

  • - Task

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

  • - The licenses files need update.
  • - There is breaking changes for older versions.
  • - It needs documentation.

Signed-off-by: tommyfufu <tommy860925@gmail.com>
@@ -395,6 +395,7 @@ There is no guaranteed resource setting for the root queue.

Maximum resources when configured place a hard limit on the size of all allocations that can be assigned to a queue at any point in time.
A maximum resource can be set to 0 which makes the resource not available to the queue.
Copy link
Contributor

Choose a reason for hiding this comment

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

change from:

A maximum resource can be set to 0 which makes the resource not available to the queue.

to:
A single resource type within the maximum resource can be set to 0 which makes that type not available to the queue.

@@ -395,6 +395,7 @@ There is no guaranteed resource setting for the root queue.

Maximum resources when configured place a hard limit on the size of all allocations that can be assigned to a queue at any point in time.
A maximum resource can be set to 0 which makes the resource not available to the queue.
However, setting all maximum resources to zero will be ignored and logged for debugging.
Copy link
Contributor

Choose a reason for hiding this comment

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

Change from:

However, setting all maximum resources to zero will be ignored and logged for debugging.

to:
However, a maximum resource with all resource types set to 0 will be ignored, not set on the queue, and logged for debugging.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just noticed that setting all the maximum resource to 0 does not simply get ignored.
If a maximum resource setting already exists in the queue, the previous setting will be cleared.

This behavior is not documented too.

https://github.com/apache/yunikorn-core/blob/ff654b2dd9038c1b026d99e553f79cd32e3d4de7/pkg/scheduler/objects/queue.go#L407-L408

Copy link
Contributor

Choose a reason for hiding this comment

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

ah yes that is correct, that was introduced via YUNIKORN-2169 to allow removing a quota. if the queue is configured and no longer has a quota.
Good catch. We need to add that behaviour as it was a fix requested but never documented.

@wilfred-s wilfred-s self-requested a review April 30, 2024 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants