feat(orchestrator): a seam for taking queued executions off the launch path - #346
Open
yuechao-qin wants to merge 1 commit into
Open
feat(orchestrator): a seam for taking queued executions off the launch path#346yuechao-qin wants to merge 1 commit into
yuechao-qin wants to merge 1 commit into
Conversation
…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
|
|
||
| def intercept(self, *, session: orm.Session, execution: bts.ExecutionNode) -> bool: | ||
| """True if this execution was taken over and must not launch; False to continue.""" | ||
| ... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a generic seam to the orchestrator so a downstream implementation can take a queued
execution off the launch path, and makes
UNINITIALIZEDmean 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
QUEUEDonly, notUNINITIALIZEDtooorchestrator_sql.py:126ORDER BYthe same low-id node is picked every time, spending the whole 2–3/sec sweep budget on one parked executionQueuedExecutionInterceptorProtocolorchestrator_sql.py:42Truemeans "I own this execution": the implementation sets whatever status it wants and commits, and the orchestrator makes no assumption about whichqueued_execution_interceptor=NoneonOrchestratorService_Sqlorchestrator_sql.py:60orchestrator_sql.py:626ContainerExecutionStatusdocstring;# Remove→# Parked by an interceptor; not sweptbackend_types_sql.py:14Tests
tests/test_orchestrator_sql.py, 9 passing in the file and 473 in the suite.UNINITIALIZED) node is not selected, and is left exactly as foundQUEUEDnode still is — narrowing the selector did not break the sweepTrue: no launch, no container, its status survivesFalse, and no interceptor at all: launches as todayKnown gap, deliberately left to the caller
An execution parked at
UNINITIALIZEDno longer sees the run-levelTERMINATEDflag, becausethe 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.