Skip to content

Storage draining rejects payload uploads needed to finish already-leased work #146

Closed

Description

@rmcdaniel

Confirmed Failure

Server 2.3.4 + PHP SDK 2.0.8 cannot finish an already-leased activity with a large result once storage admission enters draining.

The acknowledgement endpoint is deliberately allowed to drain, but the SDK first externalizes the result through POST /api/external-payloads/v1. That request is refused before the acknowledgement can be sent:

{"reason":"storage_pressure","storage_state":"draining","retryable":true,"retry_after_seconds":5,"request_admitted":false}

HTTP status is 503. The activity remains running; repeatedly retrying the upload cannot resolve the admission mismatch. Confirmed with actual published artifacts and a valid Avro payload encoding a 1,572,608-byte result, not malformed input.

Reproduce

  1. Run Server with a valid native storage-admission observation and namespace-owned external payload storage.
  2. Start a PHP SDK activity and hold it after the runtime grants its task lease.
  3. Transition storage to draining while preserving operational write headroom.
  4. Return a result exceeding the namespace externalization threshold.
  5. Observe repeated rejected external uploads and no committed activity acknowledgement.

EnforceStorageAdmission::DRAIN_ACTIONS permits task completions but not RuntimeExternalPayloadController@store. RuntimePayloadUploads::request() uploads selected fields before sending the owning completion. The upload currently has no task-lease binding.

Existing Contract

The rejection of unbound uploads is intentional in the current admission implementation. The earlier published SDK fixture (tests/Integration/StorageAdmission/sdk/experiment.php) explicitly expects Python uploads to remain paused while PHP/Rust inline completions drain. PHP 2.0.8's automatic externalization reaches that same restriction. This is the missing safe external-result drain path, not evidence that the original inline-drain test failed or that uploads should become unrestricted.

Update that fixture across all three current SDKs when implementing the bounded extension.

Required Outcome

  • Already-leased work can finish with external payloads while storage drains, with bounded reserve consumption.
  • Do not solve this by allowing arbitrary uploads for every worker credential. Any draining exception must prove the current namespace, task/attempt, owner and an allowed completion purpose; reject expired, foreign or otherwise unbound writes.
  • Keep new workflow/input admission closed, and keep fenced/stale observations fail-closed.
  • Preserve retry/idempotency, payload integrity, acknowledgement identity and cold-read behavior.
  • Cover every affected completion/failure/command payload path and PHP, Python and Rust transport implementations. A PHP-only workaround or forcing inline transport does not solve the contract.
  • Add focused Server/SDK regressions and rerun the published-artifact leased-result scenario.

Related: #131 (original inline acknowledgement draining), #141 (HTTP upload memory). These are completed earlier changes; this issue is the interaction with automatic runtime payload uploads.

Priority: fix this correctness gap before lower-priority performance qualification. Implementation should remain a bounded extension of the existing admission and payload protocol, not a second quota or retry system.

Activity

  1. added
    bugSomething isn't working
    priority:P1High-priority product or release risk
    on Sep 8, 2026
  2. rmcdaniel commented on Sep 8, 2026

    @rmcdaniel
    MemberAuthor

    The fix is now visible across all four repositories:

    All are drafts, not released. The SDKs now negotiate an optional completion context only after an ordinary upload is refused during draining. The retry is bound to the existing task lease and payload slot; hard fencing and ordinary client admission stay closed. The existing retry policies preserve computed activity results when upload admission pauses.

    Full SDK validation so far: PHP 633 tests / 3,609 assertions (one skipped), Python 1,387 non-integration tests (15 integration cases deselected), and Rust 211 library tests plus 23 integration/compile tests and 3 documentation tests. Type/lint/format checks passed. Server CI for the preceding draft commit is green. Fresh-process concurrency and persistence checks are being added before the end-to-end bounded-storage experiment; these unit/source results do not close this issue.

  3. rmcdaniel commented on Sep 8, 2026

    @rmcdaniel
    MemberAuthor

    The fixes are merged and published: Server 2.3.5 (PR #147), PHP SDK 2.0.9, Python SDK 2.0.4 and Rust SDK 2.0.5. Server release/first-run/Compose/Helm qualification and all SDK publication jobs passed.

    A second clean HTTP run used the actual downloaded Server image and SDKs installed from Packagist, PyPI and crates.io, with no source SDK or Server mount. All nine activity/workflow/query completion cases passed while draining, and a fresh Server process verified all retained payload bytes and closed task states. Server image index: sha256:593d88ddf6c9e53b6839836a2c5d02dd882321d590703b62275ee0e2adac460b; bundled Workflow 2.0.8. This confirms the published tuple, not only source tests.

    The remaining check is the native bounded-storage reproduction with 24 already-leased large results and cold restart. The issue remains open until that outcome is verified. This release does not itself change an operator's storage reserve or deploy any managed runtime.

  4. rmcdaniel commented on Sep 8, 2026

    @rmcdaniel
    MemberAuthor

    The published native-storage run found one remaining validation defect: all 24 activity attempts retained current leases and continued heartbeating, but their large-result uploads returned 409 external_payload_completion_lease_rejected. The new upload guard incorrectly also requires a fresh worker-roster heartbeat. A long-running activity can keep its own lease alive while the worker's polling/registration heartbeat ages, and ordinary activity completion does not impose that roster condition.

    I am correcting the upload guard to use the existing task/attempt/owner/namespace lease authority, including expiry and terminal-run checks, rather than poll eligibility. Regression coverage will include stale, draining, and absent roster entries with valid leases, alongside the existing invalid-lease and hard-fence cases. The native run is a failure, not completion evidence; this issue remains open. No Cloud production deployment occurred.

  5. rmcdaniel commented on Sep 8, 2026

    @rmcdaniel
    MemberAuthor

    Fixed in Server 2.3.6 with PHP SDK 2.0.9, Python 2.0.4, and Rust 2.0.5. Lease-bound completion uploads retain namespace, task/attempt, owner, purpose, expiry, and bounded-budget checks; unrelated uploads and new starts remain closed while draining, and hard-fenced observations remain fail-closed. Source regressions cover invalid bindings, failures, completion fields, and retry identity. PR #148 also removes an independent budget-creation MySQL deadlock.

    Published verification passed: nine HTTP cases across all three SDKs and activity/workflow/query completions; the existing three-SDK storage-admission fixture including cold recovery (updated in #149); and 24 simultaneously held activity completions with distinct large payloads on fresh native quota-backed storage. All 24 retained their original attempt and one handler effect, and a fresh SDK process read every completed result after runtime/database restart. The latter is correctness and reserve-consumption evidence, not a published capacity claim.

    PR #149 is merged and its main checks pass: https://github.com/durable-workflow/server/actions/runs/34268519946 . No further Server release is required for that test-only follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority:P1High-priority product or release risk

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions