Skip to content

Proto field additions for JiT flow#2182

Merged
rafal-hawrylak merged 1 commit into
mainfrom
pr4a-proto-fields
Jul 2, 2026
Merged

Proto field additions for JiT flow#2182
rafal-hawrylak merged 1 commit into
mainfrom
pr4a-proto-fields

Conversation

@rafal-hawrylak

@rafal-hawrylak rafal-hawrylak commented May 27, 2026

Copy link
Copy Markdown
Collaborator

Part of #2110 - [Integrate JiT compilation into CLI]

@rafal-hawrylak rafal-hawrylak requested a review from a team as a code owner May 27, 2026 22:14
@rafal-hawrylak rafal-hawrylak requested review from andrzej-grudzien and removed request for a team May 27, 2026 22:14
@rafal-hawrylak rafal-hawrylak force-pushed the pr4a-proto-fields branch 3 times, most recently from 6c8b2c8 to 121879a Compare May 28, 2026 13:44
Comment thread protos/execution.proto
// 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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so summing up, you will do the following:

  1. Have a global overarching timeout that covers the whole run. Once it is reached, whatever is running is terminated.
  2. 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").

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, udpated

Comment thread protos/jit.proto Outdated
Comment thread protos/execution.proto
Comment thread protos/execution.proto Outdated
Comment thread protos/core.proto Outdated

@ikholopov-omni ikholopov-omni left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments

@rafal-hawrylak rafal-hawrylak force-pushed the pr4a-proto-fields branch 3 times, most recently from 88b974c to 0fa3c01 Compare July 1, 2026 20:54
Comment thread protos/execution.proto
// 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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so summing up, you will do the following:

  1. Have a global overarching timeout that covers the whole run. Once it is reached, whatever is running is terminated.
  2. 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").

@rafal-hawrylak rafal-hawrylak merged commit b3ceda5 into main Jul 2, 2026
11 checks passed
@rafal-hawrylak rafal-hawrylak deleted the pr4a-proto-fields branch July 2, 2026 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants