-
Notifications
You must be signed in to change notification settings - Fork 43
[337] Add logic to run child processes at lower priority than parent #142
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
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
6a12faa
Add logic to run child processes nicer than parent
ionphractal 2ee7651
Move setting priority to a dedicated function
ionphractal 61add5d
Add linux tests for setting priority
ionphractal 4042fbe
Prepare windows tests for setting priority
ionphractal a597e68
Rename field and lint
ionphractal 6ae37f9
Change windows script execution
ionphractal 604deae
Add missing newline in expectation
ionphractal f4ddb3b
Use BelowNormal instead of Idle for Windows priority
neddp d15394a
Use consistent log tag in LowerProcessPriority
neddp 5161522
Make lowerProcessPriority unexported
neddp 0eb2b83
Include pid and error details in log messages
neddp 6c73c1f
Cap test priority expectation at 19
neddp 8cffe35
Fix test flakiness with sleep before reading priority
neddp 57b463e
Support SpawnWithLowerPriority in RunComplexCommandAsync
neddp 244989b
Improve SpawnWithLowerPriority documentation
neddp 6ff4e5b
Add async tests for SpawnWithLowerPriority
neddp 8294573
Inline process priority logic, drop hekmon/processpriority dependency
neddp e47c162
Narrow OpenProcess access masks to least privilege
neddp ca48643
Tests: invoke bash explicitly instead of executing temp scripts
neddp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| //go:build !windows | ||
|
|
||
| // Inspired by github.com/hekmon/processpriority (MIT, Copyright 2024 Edouard Hur). | ||
| // Reimplemented inline to avoid the external dependency. | ||
|
|
||
| package system | ||
|
|
||
| import ( | ||
| "os" | ||
| "syscall" | ||
| ) | ||
|
|
||
| // getProcessPriority returns the nice value of the process with the given pid. | ||
| func getProcessPriority(pid int) (int, error) { | ||
| // syscall.Getpriority returns the "kernel nice" (20 - nice), so we convert. | ||
| // See https://linux.die.net/man/2/getpriority | ||
| knice, err := syscall.Getpriority(syscall.PRIO_PROCESS, pid) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| nice := (knice - 20) * -1 | ||
| return nice, nil | ||
| } | ||
|
|
||
| // setProcessPriority sets the nice value of the process with the given pid. | ||
| func setProcessPriority(pid int, nice int) error { | ||
| return syscall.Setpriority(syscall.PRIO_PROCESS, pid, nice) | ||
| } | ||
|
|
||
| // lowerProcessPriority sets the child process nice value to parent + 5, clamped at 19. | ||
| func (r execCmdRunner) lowerProcessPriority(logTag string, processPid int) error { | ||
| parentPid := os.Getpid() | ||
|
|
||
| parentNice, err := getProcessPriority(parentPid) | ||
| if err != nil { | ||
| r.logger.Error(logTag, "Error getting priority of the current process (pid %d): %s", parentPid, err) | ||
| return err | ||
| } | ||
| r.logger.Debug(logTag, "Current process nice value is %d", parentNice) | ||
|
|
||
| childNice := min(parentNice+5, 19) | ||
| r.logger.Debug(logTag, "Setting child process (pid %d) nice value to %d", processPid, childNice) | ||
|
|
||
| if err = setProcessPriority(processPid, childNice); err != nil { | ||
| r.logger.Error(logTag, "Error setting priority on child process (pid %d): %s", processPid, err) | ||
| } | ||
| return err | ||
| } |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| //go:build windows | ||
|
|
||
| // Inspired by github.com/hekmon/processpriority (MIT, Copyright 2024 Edouard Hur). | ||
| // Reimplemented inline to avoid the external dependency. | ||
|
|
||
| package system | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "os" | ||
|
|
||
| "golang.org/x/sys/windows" | ||
| ) | ||
|
|
||
| const ( | ||
| // Windows process priority classes. | ||
| // https://learn.microsoft.com/en-us/windows/win32/procthread/scheduling-priorities | ||
| winPriorityBelowNormal = 0x4000 // BELOW_NORMAL_PRIORITY_CLASS | ||
| ) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| // getProcessPriority returns the priority class of the process with the given pid. | ||
| func getProcessPriority(pid int) (int, error) { | ||
| handle, err := windows.OpenProcess(windows.PROCESS_QUERY_INFORMATION, false, uint32(pid)) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("failed to open process: %w", err) | ||
| } | ||
| defer windows.CloseHandle(handle) //nolint:errcheck | ||
|
|
||
| priority, err := windows.GetPriorityClass(handle) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("failed to get priority class: %w", err) | ||
| } | ||
| return int(priority), nil | ||
| } | ||
|
|
||
| // setProcessPriority sets the priority class of the process with the given pid. | ||
| func setProcessPriority(pid int, priority int) error { | ||
| handle, err := windows.OpenProcess(windows.PROCESS_SET_INFORMATION, false, uint32(pid)) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to open process: %w", err) | ||
| } | ||
| defer windows.CloseHandle(handle) //nolint:errcheck | ||
|
|
||
| if err = windows.SetPriorityClass(handle, uint32(priority)); err != nil { | ||
| return fmt.Errorf("failed to set priority class: %w", err) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // lowerProcessPriority sets the child process priority class to BelowNormal. | ||
| func (r execCmdRunner) lowerProcessPriority(logTag string, processPid int) error { | ||
| parentPid := os.Getpid() | ||
|
|
||
| parentPriority, err := getProcessPriority(parentPid) | ||
| if err != nil { | ||
| r.logger.Error(logTag, "Error getting priority of the current process (pid %d): %s", parentPid, err) | ||
| return err | ||
| } | ||
| r.logger.Debug(logTag, "Current process priority class is %d", parentPriority) | ||
|
|
||
| r.logger.Debug(logTag, "Setting child process (pid %d) priority to BelowNormal", processPid) | ||
| if err = setProcessPriority(processPid, winPriorityBelowNormal); err != nil { | ||
| r.logger.Error(logTag, "Error setting priority on child process (pid %d): %s", processPid, err) | ||
| } | ||
| return err | ||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.