Repository navigation
Conversation
This was referenced Oct 11, 2026
This was referenced Oct 12, 2026
nikhilbirmiwal-pplx
approved these changes
Oct 12, 2026
zibi-pplx
added a commit
that referenced
this pull request
Oct 12, 2026
Frequent teardown polls can immediately restart a failed driver, repeatedly joining the same blocked upload or retrying a failed dependency. Persist a retry deadline on the generation's existing teardown job and defer admission until it expires. Waiting occupies neither a driver task nor a semaphore permit, and daemon restarts preserve the deadline. The delay is a named 30-second constant and applies to all failed driver attempts that leave an active operation. This deliberately bounds transient outage retries as well as permanent blocks, at the cost of up to one cooldown before retrying after recovery. Existing terminal cleanup and newer-generation admission retain their behavior. There is no upload reopening, authority change, or schema migration. Sandbox-manager-only rollout. Severity: medium, because unpaced failures amplify load and logs while teardown is already blocked. Stacked directly on #281. Backup-admission changes are maintained in an independent PR; this stack contains only worker teardown fixes. The regression has its own test name and exercises repeated polls, released driver capacity, and retry after expiry. Prior validation: the admission regression failed before pacing was added. All 1,560 sandbox-manager library tests, affected-package all-target clippy with warnings denied, formatting, and Gitleaks passed. Tests ran on macOS; Linux workspace CI and Firecracker end-to-end validation were not run locally. Stack-split validation: the rebased patch is byte-for-byte identical to the reviewed patch. Both sandbox-manager teardown backup regressions passed with #285 directly on #281 (2 passed, no skips). AI assistance: Codex contributed the implementation, tests, and description.
zibi-pplx
added a commit
that referenced
this pull request
Oct 12, 2026
A fresh backup attempt can be requested after execution retirement has cleared placement. This proposal derives a fallback source from the last scheduled execution's node name and matching pending artifact ID/kind, subject to lifecycle, lease, active-execution, and current-artifact-ID checks. It locks the sandbox through attempt registration and authenticates through the same transaction connection. Existing attempts retain their persisted source. Design review in progress; not ready to merge. Review findings about conflicting artifact kind, same-name node replacement, artifact provenance, and worker-authored inventory remain unresolved. Passing the existing tests does not establish that this authorization design is sufficient. The proposed schema/identity rework is not part of this patch. This is an independent patchstack based on main. The approved metadata-replacement and retry-pacing changes are maintained separately in #281 and #285. This patch changes the internal API only, has no migration or protobuf change, and does not reopen terminal backup jobs automatically. Prior validation: all 244 internal API library tests passed with required Docker/Postgres dependencies, including the existing 11-case authority matrix, placement locking, persisted-source replay, concurrent Begin idempotency, and missing-source Finalize refusal. Formatting, all-target clippy with warnings denied, and Gitleaks passed before the stack split. The review subsequently identified cases missing from that matrix. Linux/Firecracker end-to-end behavior was not validated locally. Stack-split validation: the rebased patch is byte-for-byte identical to the previously reviewed patch. The existing retired-backup authority regression passed against the new main base with required Postgres dependencies (1 passed, no skips); the unresolved review cases above are not covered by that result. AI assistance: Codex contributed the implementation, tests, and description.
zibi-pplx
added a commit
that referenced
this pull request
Oct 12, 2026
Frequent teardown polls can immediately restart a failed driver, repeatedly joining the same blocked upload or retrying a failed dependency. Persist a retry deadline on the generation's existing teardown job and defer admission until it expires. Waiting occupies neither a driver task nor a semaphore permit, and daemon restarts preserve the deadline. The delay is a named 30-second constant and applies to all failed driver attempts that leave an active operation. This deliberately bounds transient outage retries as well as permanent blocks, at the cost of up to one cooldown before retrying after recovery. Existing terminal cleanup and newer-generation admission retain their behavior. There is no upload reopening, authority change, or schema migration. Sandbox-manager-only rollout. Severity: medium, because unpaced failures amplify load and logs while teardown is already blocked. Stacked directly on #281. Backup-admission changes are maintained in an independent PR; this stack contains only worker teardown fixes. The regression has its own test name and exercises repeated polls, released driver capacity, and retry after expiry. Prior validation: the admission regression failed before pacing was added. All 1,560 sandbox-manager library tests, affected-package all-target clippy with warnings denied, formatting, and Gitleaks passed. Tests ran on macOS; Linux workspace CI and Firecracker end-to-end validation were not run locally. Stack-split validation: the rebased patch is byte-for-byte identical to the reviewed patch. Both sandbox-manager teardown backup regressions passed with #285 directly on #281 before the main rebase (2 passed, no skips). Rebasing the two-PR stack onto main at 37e5d74 also preserved both patches byte-for-byte without conflicts. AI assistance: Codex contributed the implementation, tests, and description.
Frequent teardown polls can immediately restart a failed driver, repeatedly joining the same blocked upload or retrying a failed dependency. Persist a retry deadline on the generation's existing teardown job and defer admission until it expires. Waiting occupies neither a driver task nor a semaphore permit, and daemon restarts preserve the deadline. The delay is a named 30-second constant and applies to all failed driver attempts that leave an active operation. This deliberately bounds transient outage retries as well as permanent blocks, at the cost of up to one cooldown before retrying after recovery. Existing terminal cleanup and newer-generation admission retain their behavior. There is no upload reopening, authority change, or schema migration. Sandbox-manager-only rollout. Severity: medium, because unpaced failures amplify load and logs while teardown is already blocked. Stacked directly on #281. Backup-admission changes are maintained in an independent PR; this stack contains only worker teardown fixes. The regression has its own test name and exercises repeated polls, released driver capacity, and retry after expiry. Prior validation: the admission regression failed before pacing was added. All 1,560 sandbox-manager library tests, affected-package all-target clippy with warnings denied, formatting, and Gitleaks passed. Tests ran on macOS; Linux workspace CI and Firecracker end-to-end validation were not run locally. Stack-split validation: the rebased patch is byte-for-byte identical to the reviewed patch. Both sandbox-manager teardown backup regressions passed with #285 directly on #281 before the main rebase (2 passed, no skips). Rebasing the two-PR stack onto main at 37e5d74 also preserved both patches byte-for-byte without conflicts. Validation on the rebased main stack: both teardown backup regressions passed again (2 passed, no skips). AI assistance: Codex contributed the implementation, tests, and description.
zibi-pplx
added a commit
that referenced
this pull request
Oct 12, 2026
A worker may need a fresh backup attempt after its execution has retired. Require an exact earlier registration from the same source incarnation for that retry. Refuse first registration after retirement, even when inventory and the current-artifact pin match; worker-reported inventory grants no authority. First registration on active or target-only placement must match the active execution's recorded Node UID. Persist that UID per attempt and check it again on replay and finalize, holding sandbox and node locks through the catalog write. Reject conflicting artifact kind, origin, or snapshot name. Retries inherit the earliest verified registration timestamp so an older artifact cannot become fresh restore evidence after relocation. Depends on the separate additive-schema and identity-writer PRs. Deploy schema, then both execution writers, then enforcement; drain old in-flight uploads first. Historical attempts without UIDs cannot finalize under the new policy. Deploy exact-Ready reuse from #281 before enforcement to avoid unnecessary reuploads of restored artifacts. Keep this stack separate from approved #281 -> #285. Registered-address authentication and credential vending remain separate boundaries. Registry convergence is required to detect node replacement; this is not caller-carried incarnation authentication. No job reset, deployment, or live recovery is included. This PR remains draft for architecture review. Validation: 416 selected PostgreSQL/controller/storage tests passed with required PostgreSQL and Redis, including the 24-case admission matrix, prior-registration freshness, replay identity, terminal transitions, and pool adoption. Formatting and repository policy checks passed. Affected-package all-target clippy passed with warnings denied. The final run passed including the transaction-lock regression; nextest reported one child-output leak in pooled-create case 1. Gitleaks found no disclosures in the complete diff. The complete CI lint runner stops on macOS at its Linux-only Wild prerequisite; Linux/Firecracker end-to-end behavior remains unverified. AI assistance: Codex contributed implementation, tests, and description.
zibi-pplx
added a commit
that referenced
this pull request
Oct 12, 2026
A worker may need a fresh backup attempt after its execution has retired. Require an exact earlier registration from the same source incarnation for that retry. Refuse first registration after retirement, even when inventory and the current-artifact pin match; worker-reported inventory grants no authority. First registration on active or target-only placement must match the active execution's recorded Node UID. Persist that UID per attempt and check it again on replay and finalize, holding sandbox and node locks through the catalog write. Reject conflicting artifact kind, origin, or snapshot name. Retries inherit the earliest verified registration timestamp so an older artifact cannot become fresh restore evidence after relocation. Depends on schema #318 and identity writers #319 in native GitHub stack #320. Deploy schema, then both execution writers, then enforcement; drain old in-flight uploads first. Historical attempts without UIDs cannot finalize under the new policy. Deploy exact-Ready reuse from #281 before enforcement to avoid unnecessary reuploads of restored artifacts. Keep this stack separate from approved #281 -> #285. Registered-address authentication and credential vending remain separate boundaries; credential-vending follow-up is #321. Registry convergence is required to detect node replacement; this is not caller-carried incarnation authentication. No job reset, deployment, or live recovery is included. This PR remains draft for architecture review. Validation: 416 selected PostgreSQL/controller/storage tests passed with required PostgreSQL and Redis, including the 24-case admission matrix, prior-registration freshness, replay identity, terminal transitions, and pool adoption. Formatting and repository policy checks passed. Affected-package all-target clippy passed with warnings denied. The final run passed including the transaction-lock regression; nextest reported one child-output leak in pooled-create case 1. Gitleaks and public-disclosure review had no findings in the changed scope. The complete CI lint runner stops on macOS at its Linux-only Wild prerequisite; Linux/Firecracker end-to-end behavior remains unverified. AI assistance: Codex contributed implementation, tests, and description.
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.
Frequent teardown polls can immediately restart a failed driver, repeatedly joining the same blocked upload or retrying a failed dependency. Persist a retry deadline on the generation's existing teardown job and defer admission until it expires. Waiting occupies neither a driver task nor a semaphore permit, and daemon restarts preserve the deadline.
The delay is a named 30-second constant and applies to all failed driver attempts that leave an active operation. This deliberately bounds transient outage retries as well as permanent blocks, at the cost of up to one cooldown before retrying after recovery. Existing terminal cleanup and newer-generation admission retain their behavior. There is no upload reopening, authority change, or schema migration. Sandbox-manager-only rollout. Severity: medium, because unpaced failures amplify load and logs while teardown is already blocked.
Stacked directly on #281. Backup-admission changes are maintained in an independent PR; this stack contains only worker teardown fixes. The regression has its own test name and exercises repeated polls, released driver capacity, and retry after expiry.
Prior validation: the admission regression failed before pacing was added. All 1,560 sandbox-manager library tests, affected-package all-target clippy with warnings denied, formatting, and Gitleaks passed. Tests ran on macOS; Linux workspace CI and Firecracker end-to-end validation were not run locally.
Stack-split validation: the rebased patch is byte-for-byte identical to the reviewed patch. Both sandbox-manager teardown backup regressions passed with #285 directly on #281 before the main rebase (2 passed, no skips). Rebasing the two-PR stack onto main at 37e5d74 also preserved both patches byte-for-byte without conflicts.
Validation on the rebased main stack: both teardown backup regressions passed again (2 passed, no skips).
AI assistance: Codex contributed the implementation, tests, and description.
Stack created with Sapling. Best reviewed with ReviewStack.