Skip to content

feat(orchestrator): a seam for taking queued executions off the launch path - #346

Open
yuechao-qin wants to merge 1 commit into
masterfrom
ycq/gsd-proton-quota-group-upstream-seam
Open

feat(orchestrator): a seam for taking queued executions off the launch path#346
yuechao-qin wants to merge 1 commit into
masterfrom
ycq/gsd-proton-quota-group-upstream-seam

Conversation

@yuechao-qin

Copy link
Copy Markdown
Collaborator

What

Adds a generic seam to the orchestrator so a downstream implementation can take a queued
execution off the launch path, and makes UNINITIALIZED mean parked rather than legacy.

Nothing here mentions quotas: Shopify's quota groups are the first user of the seam, and they
live entirely in oasis-backend.

How it works

  sweep picks a QUEUED node
        │
        ├─ inputs ready? cache hit? cancelled?      (unchanged)
        │
        ├─ interceptor.intercept(session, execution)
        │        │
        │        ├─ True  ──► it committed its own status. We return. No container.
        │        └─ False ──► fall through
        │
        └─ create container execution               (unchanged)
Change File Why
Sweep selects QUEUED only, not UNINITIALIZED too orchestrator_sql.py:126 Parking must actually hide a node. Otherwise it is re-selected next tick, redoes the work above the gate and re-parks — and with no ORDER BY the same low-id node is picked every time, spending the whole 2–3/sec sweep budget on one parked execution
QueuedExecutionInterceptor Protocol orchestrator_sql.py:42 The seam. Returning True means "I own this execution": the implementation sets whatever status it wants and commits, and the orchestrator makes no assumption about which
One keyword-only queued_execution_interceptor=None on OrchestratorService_Sql orchestrator_sql.py:60 Every existing caller is unaffected
The call site, after the cancel check and before container creation orchestrator_sql.py:626 The latest point at which the execution is known to be launchable
ContainerExecutionStatus docstring; # Remove# Parked by an interceptor; not swept backend_types_sql.py:14 The status is no longer a leftover

Tests

tests/test_orchestrator_sql.py, 9 passing in the file and 473 in the suite.

  • a parked (UNINITIALIZED) node is not selected, and is left exactly as found
  • a QUEUED node still is — narrowing the selector did not break the sweep
  • interceptor returns True: no launch, no container, its status survives
  • interceptor returns False, and no interceptor at all: launches as today

Known gap, deliberately left to the caller

An execution parked at UNINITIALIZED no longer sees the run-level TERMINATED flag, because
the sweep no longer looks at it. Cancelling a run whose parked nodes have no live sibling in the
same group therefore leaves those nodes non-terminal until something requeues them.

This is a property of parking, not of this diff — nothing upstream parks today — so the duty
sits with whoever installs an interceptor: they must un-park on the cancel path. Shopify's
implementation does so, and the four cancellation scenarios are covered by its own tests. Say
the word if you would rather the orchestrator kept selecting parked rows that belong to a
terminated run, and I will add it here instead.

…h path

Adds `QueuedExecutionInterceptor`, a Protocol the orchestrator consults after
the cancellation check and before creating a container. Returning True means
the implementation owns the execution: it sets whatever status it wants and
commits, and the orchestrator does not launch.

`OrchestratorService_Sql` gains one keyword-only `queued_execution_interceptor`
parameter defaulting to None, so every existing caller is unaffected.

The queued sweep now selects QUEUED only, not UNINITIALIZED too, which makes
UNINITIALIZED a parked state that is actually hidden. Without this a parked
execution is re-selected on the next tick, redoes the work above the gate and
re-parks -- and with no ORDER BY the same low-id row is picked every time,
spending the whole sweep budget on one parked execution.

Assisted-By: devx/20d7f01c-ddc9-41c5-8b3e-5e921c5b7717
@yuechao-qin
yuechao-qin requested a review from a team August 26, 2026 22:51
@yuechao-qin
yuechao-qin requested a review from Ark-kun as a code owner August 26, 2026 22:51

def intercept(self, *, session: orm.Session, execution: bts.ExecutionNode) -> bool:
"""True if this execution was taken over and must not launch; False to continue."""
...
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.

1 participant