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 balloon type specific pinMemory option #451

Merged
merged 4 commits into from
Dec 17, 2024

Conversation

askervin
Copy link
Collaborator

This option enables overriding policy level pinMemory option in balloon types.

Use cases:

  1. Enable pinning memory of latency critical container to the lowest latency nodes while leaving all other containers completely unpinned.
  2. Allow certain containers to access all memory nodes while memory of all other containers would be pinned.

@klihub klihub requested a review from fmuyassarov December 17, 2024 07:46
Copy link
Collaborator

@klihub klihub left a comment

Choose a reason for hiding this comment

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

Looks fine, apart from a formatting error which needs fixing. Also, I think we should try to keep libmem in the loop about total memory usage. The container either might have some pinning already calculated by some other component or it will be pinned to all memory nodes. In either case, its max. memory consumption would need to be accounted where it is going to be pinned, even if that pinning has not been selected by us. But it is perfectly fine to address that in another PR.

if (blnDefPinMemory != nil) {
pinMemory = *blnDefPinMemory
}
if pinMemory {
Copy link
Collaborator

@klihub klihub Dec 17, 2024

Choose a reason for hiding this comment

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

I think we should let also libmem know about the new container consuming memory, even if we don't ask libmem to determine the pinning according to its own taste. Otherwise even in theory it cannot determine correctly when a node is in danger of running out of memory.

@askervin askervin force-pushed the 5YX_balloontype_pinmem branch from a350524 to b8d7f31 Compare December 17, 2024 09:49
pinMemory in balloon type enables overriding pinMemory policy level
option in balloon type level.

Signed-off-by: Antti Kervinen <[email protected]>
@askervin askervin force-pushed the 5YX_balloontype_pinmem branch from b8d7f31 to 9fa8220 Compare December 17, 2024 10:12
@klihub klihub self-requested a review December 17, 2024 10:13
Copy link
Collaborator

@klihub klihub left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Collaborator

@kad kad left a comment

Choose a reason for hiding this comment

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

lgtm

@klihub klihub merged commit 109d59c into containers:main Dec 17, 2024
9 checks passed
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.

3 participants