Skip to content

Proto field additions for JiT flow #2182

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.

Sign up for GitHub

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
rafal-hawrylak wants to merge 1 commit into
base: main
Choose a base branch
from
+21 −1
Open

Proto field additions for JiT flow #2182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion protos/core.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,16 @@ message CompileConfig {
// Project configuration overrides.
ProjectConfig project_config_override = 3;

// Override compilation timeout settings.
// Override top-level compilation timeout (the single compile worker that
// runs once per invocation). Does not bound per-model JiT compilation;
// see jit_timeout_millis.
int32 timeout_millis = 6;

// Per-model JiT compilation worker timeout. Each model with JiT code gets
// its own fresh budget; this is per-model, not a shared budget across the
// compile.
int32 jit_timeout_millis = 13;

@ikholopov-omni ikholopov-omni Jun 3, 2026

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.

Same as protos/execution.proto.

@rafal-hawrylak rafal-hawrylak Jun 19, 2026

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.

Same reply as #2182 (comment)


Extension extension = 10;

// Whether to emit compilation debug logs.
Expand Down
11 changes: 11 additions & 0 deletions protos/execution.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,16 @@ message RunConfig {
bool include_dependencies = 3;
bool include_dependents = 11;
bool full_refresh = 2;

// Wall-clock deadline for the entire run. When hit, all in-flight actions are
// 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

@ikholopov-omni ikholopov-omni Jun 3, 2026

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.

@rafal-hawrylak rafal-hawrylak Jun 19, 2026

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.

// its own fresh budget; this is per-model, not a shared budget across the
// run. Falls back to DEFAULT_JIT_COMPILATION_TIMEOUT_MILLIS when unset.
int32 jit_timeout_millis = 10;

// For internal use only, will be removed at a later date.
bool disable_set_metadata = 9;

Expand Down Expand Up @@ -46,6 +54,8 @@ message ExecutionAction {

string jit_code = 12;

bool disabled = 13;

@ikholopov-omni ikholopov-omni Jun 3, 2026

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 this now? I don't think it was required before and don't understand why we need it now.

@rafal-hawrylak rafal-hawrylak Jun 19, 2026

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.

With the introduction of JiT, actions are compiled at execution time, meaning both enabled and disabled JiT actions initially have an empty tasks array. To prevent the runner from compiling and executing disabled JiT actions, the explicit disabled flag is required on the ExecutionAction proto so the runner can identify and skip them.


reserved 1, 3, 7;
}

Expand Down Expand Up @@ -118,6 +128,7 @@ message TaskResult {
string error_message = 2;
Timing timing = 3;
ExecutionMetadata metadata = 4;
string compiled_sql = 5;

@ikholopov-omni ikholopov-omni Jun 3, 2026

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 here? (I have some guess, but please add a comment explaining when it is populated).

@rafal-hawrylak rafal-hawrylak Jun 19, 2026 •
edited
Loading

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.

compiled_sql is needed to expose the dynamically compiled SQL during dry-runs (--dry-run). Because JiT actions are compiled right before execution, their generated SQL is only available at runtime. Populating compiled_sql in TaskResult allows dry-run executions to output the final compiled SQL to the user. I will add a code comment explaining that this is populated during dry-run executions.

}

message TestResult {
Expand Down
2 changes: 2 additions & 0 deletions protos/jit.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ message JitCompilationRequest {
repeated string file_paths = 6;
// Current execution information for introspection.
RunningExecutionData execution_data = 7;
// File name where the target is defined.
string file_name = 8;

@ikholopov-omni ikholopov-omni Jun 3, 2026

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? Is it just for better error handling? (having it in PR description would be helpful)

@rafal-hawrylak rafal-hawrylak Jun 19, 2026

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.

The file_name property is required to enable correct resolution of relative module imports (via require / import) inside the sandboxed VM during JiT compilation. Without it, the VM cannot determine the correct directory context of the SQLX/JS source code to resolve relative paths to dependencies in the project structure.

}

// JiT compilation response.
Expand Down
Loading