-
Notifications
You must be signed in to change notification settings - Fork 2.1k
core: timeouts for batch jobs #27803
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
Open
pkazmierczak
wants to merge
37
commits into
main
Choose a base branch
from
f-timeouts-for-batch-jobs
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
605395d
core: support batch job timeouts via new field max_run_duration
pkazmierczak 2388417
refine batch timeout watcher
pkazmierczak 617fe2b
client status
pkazmierczak 1f4c4db
integration-style test
pkazmierczak 180aea3
event stream
pkazmierczak 5fff828
AllocTimeoutReasonMaxRunDuration
pkazmierczak 8fbfff3
batch timeout watcher improvements
pkazmierczak 697c5c3
added batchtimeout package to test-core.json
pkazmierczak a31525a
move to allocrunner
pkazmierczak 2a7fc3b
remove nomad/batchtimeout
pkazmierczak e9d30e7
timer tweaks
pkazmierczak d560d47
max_run_duration_hook
pkazmierczak 737f23d
TestJobs_ApiJobToStructsJob
pkazmierczak 48a9652
don't need that interface
pkazmierczak eda5a4f
it's actually not a hook
pkazmierczak ab7565d
refinements
pkazmierczak c44ba70
corrections
pkazmierczak 9abba4f
test refactror
pkazmierczak 0aecff5
tidying up
pkazmierczak f454526
more tidying up
pkazmierczak 9b735a0
bugfix
pkazmierczak 7e0509a
unify fully running since
pkazmierczak ea2f1ad
hook alloc can never be nil
pkazmierczak f996731
even stream corrections
pkazmierczak a521239
desired status comment
pkazmierczak 2f420ac
allocrunner unnecessary copy fix
pkazmierczak ae2d08a
fix UTC and interface
pkazmierczak bc6344c
max_run_duration cleanups (thanks for the comments @mismithhisler)
pkazmierczak f246b59
client: enforce max_run_duration regardless of task state (#27827)
pkazmierczak 3b6f4df
allow for updates to max_run_duration during job run
pkazmierczak 53f6210
metrics
pkazmierczak 7841eda
better logs
pkazmierczak ce45ae8
fix clock arming bug
pkazmierczak 6380561
don't start poststop tasks if the timeout has passed
pkazmierczak 879219a
TestMaxRunDurationHook_EmitMetrics correction
pkazmierczak 6a6e6a6
TestServiceSched_JobModify_MaxRunDuration_InPlace fix
pkazmierczak e34dbc9
yet another fix to TestMaxRunDurationHook_EmitMetrics
pkazmierczak 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
Some comments aren't visible on the classic Files Changed page.
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we'd regret naming this
ttl? It is literally the amount time we allow the allocation to live, but it might be too ambiguous of a name for thegroupblock. You could interpret thegroup.ttlas including time sitting in the eval queue as well or something, whereas therunaspect of this makes it more obvious.So I think this is fine, but since jobspec fields are our core user interface, I think they're worth careful consideration. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I don't know about this.
ttlhas a loaded meaning in my opinion. We can think of a better name for this setting, sure, but I don't think it should bettl.