Proto field additions for JiT flow#2182
Conversation
6c8b2c8 to
121879a
Compare
| // cancelled. Does not bound per-model JiT compilation; see jit_timeout_millis. | ||
| int32 timeout_millis = 7; | ||
|
|
||
| // Per-model JiT compilation worker timeout. Each action with JiT code gets |
There was a problem hiding this comment.
Why do we need it? Why not just use a generic timeout_millis? Scoping JiT stage of execution of timeout_millis looks like a broken contract to me.
There was a problem hiding this comment.
timeout_millis specifies the wall-clock deadline for the entire execution run (which can be hours). JiT compilation, however, is executed on a separate worker thread for each individual model immediately before its tasks run. If a single model's compilation hangs, allowing it to consume the entire timeout_millis would block the pipeline and starve other models. Each model needs its own small, independent compilation budget (defaulting to 60s) that resets per action.
There was a problem hiding this comment.
Ok. Why not only respect timeout_millis deadline and don't set per-jit task deadline at all - instead terminate them once an overarching timeout_millis is terminated?
There was a problem hiding this comment.
My idea here was to have separate budget for the workers so if among hundreds there is only one that hung or is long-running, all the rest would have a chance to be successfully executed. With global deadline this single action will eat the whole budget.
There was a problem hiding this comment.
I don't think I communicated my idea well. Timeouts shouldn't be budgets. They are not additive. Timeout should be a deadline. A point in time after which the whole run should ramp up the activity. If a single action is hanging - ok, others are still running in their execution context. If the general timeout is reached, we terminate whoever failed to complete execution.
There was a problem hiding this comment.
Ok, a budget is not a fortunate word here - correct - timeouts should not add up.
But lets take another, bolder example. Lets say there is a separate timeout on connection to BQ or execution in BQ - which is a common practice. Such a timeout may coexist with the global timeout. Having dedicated timeouts usually allows more graceful custom subprocesses eviction.
There was a problem hiding this comment.
Ok, so summing up, you will do the following:
- Have a global overarching timeout that covers the whole run. Once it is reached, whatever is running is terminated.
- In addition, it is possible to set JiT action timeout. If JiT action reaches it, it is marked as failed, but can free up an execution slot for other queued actions (if there is a concurrency limit hit).
Is this description full & correct now? If yes, please update comments and remove unnecessary confusing statements or terms (like "Does not bound per-model JiT compilation" or "budget").
There was a problem hiding this comment.
Yes, udpated
121879a to
e2d053d
Compare
88b974c to
0fa3c01
Compare
| // cancelled. Does not bound per-model JiT compilation; see jit_timeout_millis. | ||
| int32 timeout_millis = 7; | ||
|
|
||
| // Per-model JiT compilation worker timeout. Each action with JiT code gets |
There was a problem hiding this comment.
Ok, so summing up, you will do the following:
- Have a global overarching timeout that covers the whole run. Once it is reached, whatever is running is terminated.
- In addition, it is possible to set JiT action timeout. If JiT action reaches it, it is marked as failed, but can free up an execution slot for other queued actions (if there is a concurrency limit hit).
Is this description full & correct now? If yes, please update comments and remove unnecessary confusing statements or terms (like "Does not bound per-model JiT compilation" or "budget").
0fa3c01 to
74f9af1
Compare
Part of #2110 - [Integrate JiT compilation into CLI]