client: refactor cpuset manager initialization #14230
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shoenig
force-pushed
the
b-fix-cpuset-init
branch
from
cf2a951 to
16a1b61
Compare
shoenig
force-pushed
the
b-fix-cpuset-init
branch
from
16a1b61 to
da17f3f
Compare
shoenig
force-pushed
the
b-fix-cpuset-init
branch
from
da17f3f to
209caa4
Compare
shoenig
force-pushed
the
b-fix-cpuset-init
branch
from
209caa4 to
42f5684
Compare
shoenig
force-pushed
the
b-fix-cpuset-init
branch
from
42f5684 to
21b0614
Compare
shoenig
added
backport/1.2.x
shoenig
force-pushed
the
b-fix-cpuset-init
branch
from
21b0614 to
fe53ed2
Compare
lgfa29
approved these changes
Aug 25, 2022
ahh I dunno how I keep forgetting these! I'll add one |
This PR refactors the code path in Client startup for setting up the cpuset cgroup manager (non-linux systems not affected). Before, there was a logic bug where we would try to read the cpuset.cpus.effective cgroup interface file before ensuring nomad's parent cgroup existed. Therefor that file would not exist, and the list of useable cpus would be empty. Tasks started thereafter would not have a value set for their cpuset.cpus. The refactoring fixes some less than ideal coding style. Instead we now bootstrap each cpuset manager type (v1/v2) within its own constructor. If something goes awry during bootstrap (e.g. cgroups not enabled), the constructor returns the noop implementation and logs a warning. Fixes #14229
shoenig
force-pushed
the
b-fix-cpuset-init
branch
from
fe53ed2 to
13bd08b
Compare
lgfa29
added
backport/1.3.x
lgfa29
pushed a commit
that referenced
this pull request
Aug 25, 2022
client: refactor cpuset manager initialization
lgfa29
pushed a commit
that referenced
this pull request
Aug 25, 2022
client: refactor cpuset manager initialization Co-authored-by: Seth Hoenig <[email protected]>
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors the code path in Client startup for setting up the cpuset
cgroup manager (non-linux systems not affected).
Before, there was a logic bug where we would try to read the
cpuset.cpus.effectivecgroup interface file before ensuring nomad's parent cgroup existed. Therefore on first run that
file would not exist, and the list of usable cpus would be empty. Tasks started
thereafter would not have a value set for their
cpuset.cpusfile.The refactoring fixes some hard to parse init code. Instead we now bootstrap
each cpuset manager type (v1/v2) within its own constructor. If something goes
awry during bootstrap (e.g. cgroups not enabled), the constructor returns the
noop implementation and logs a warning.
Fixes #14229
Only 1.3.x and higher, when cgroup v2 support was added