DM-55434: BPS Provisioner Service Job May End Before finalJob#94
Conversation
faed230 to
24d4b55
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #94 +/- ##
==========================================
+ Coverage 84.78% 84.79% +0.01%
==========================================
Files 19 19
Lines 4260 4263 +3
Branches 458 458
==========================================
+ Hits 3612 3615 +3
Misses 570 570
Partials 78 78 ☔ View full report in Codecov by Harness. |
- trap SIGTERM in script and perform one final provisioner loop - keep provisionerJob alive until N consecutive allocateNodes failures
24d4b55 to
adabde5
Compare
mxk62
left a comment
mxk62
left a comment
There was a problem hiding this comment.
The changes looks fine. However, please do not forget to update package documentation! The default settings in lines 429 - 453 of doc/lsst.ctrl.bps.htcondor/userguide.rst must reflect any changes you'll make.
|
|
||
| allocating=1 | ||
| failures=0 | ||
| max_failures=10 |
There was a problem hiding this comment.
I'd think it would be better to let max_failure be a tunable parameter by introducing provisioningMaxFailures setting.
| wait $! || : | ||
| done | ||
| echo "$(date -u): Shutting down provisioningJob..." | ||
| sleep 20 |
There was a problem hiding this comment.
Again, it would be preferable to let the user adjust this value if needed. Can we use something like provisioningShutdownWaitTime to make it tunable?
| {provisioningExtraOptions} \ | ||
| '{provisioningPlatform}') | ||
|
|
||
| trap "allocating=0" SIGTERM |
There was a problem hiding this comment.
Don't we need to kill the background sleep when handling the signal too?
There was a problem hiding this comment.
It'd be good hygiene. Worst case there's a zombie sleep that lives for a time, best case htcondor signals the progress group.
mxk62
changed the title
ca4ec53 to
020c43e
Compare
- parameterize `provisioningMaxConsecutiveFailures` and `provisioningShutdownAllowanceSeconds` - ensure background sleep is signalled in trap handler - update `userguide.rst`
020c43e to
6a2bf6f
Compare
2 participants
Updates default provisioningScript to handle SIGTERM correctly and complete a final provisioning attempt at shutdown.
Script tries to stay alive until 10 consecutive provisioning failures.
Checklist
doc/changes