microsoft / microsoft/duroxide

Worker abandons work items on retryable ack failures, causing redundant activity re-execution

Open
#41 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Rust
Stars
217
Forks
61
Avg merge
2d 22h
Merged PRs (30d)
3

Description

Summary

A retryable ack_work_item failure causes the worker dispatcher to abandon the work item, which redelivers the activity and runs the handler again. The dispatcher poll loop already retries retryable provider errors on fetch_work_item; the ack path does not, and abandons immediately regardless of the error's retry classification.

This surfaces as a flaky test — session_e2e_tests::test_multi_worker_heterogeneous_config — but the flake is only the symptom. There are two distinct defects.

Defect 1 — worker abandons on retryable ack failures

src/runtime/dispatchers/worker.rs handle_activity_outcome abandons on any ack error:

Err(e) => {
    warn!(..., "worker: atomic ack failed, abandoning work item");
    let _ = rt.history_store
        .abandon_work_item(&ctx.lock_token, Some(Duration::from_millis(100)), false)
        .await;
    rt.record_activity_infra_error();
}

Compare the poll loop in the same file, which does branch on e.is_retryable() and backs off:

Err(e) if e.is_retryable() => {
    consecutive_retryable_errors += 1;
    let backoff_ms = (100 * 2_u64.pow(consecutive_retryable_errors)).min(3000);
    warn!("Error fetching work item (retryable, attempt {}): {:?}, backing off {}ms", ...);
    tokio::time::sleep(Duration::from_millis(backoff_ms)).await;
    continue;
}

Activities are documented as at-least-once (docs/durable-futures-internals.md, "Activity Guarantees: At-Least-Once Execution"), so re-execution is not a correctness violation. But re-running a completed handler because the database was contended for a few milliseconds is wasteful and avoidable — the ack transaction rolled back, so retrying in place is safe.

Defect 2 — test asserts exactly-once on an at-least-once runtime

tests/session_e2e_tests.rs:

assert_eq!(a + b, 3, "Total should be 3, got A={a} B={b}");

This asserts each of the 3 scheduled activities executed exactly once. Any ack failure or lock expiry redelivers, so the assertion is wrong independent of the storage layer. Observed failure: Total should be 3, got A=1 B=3.

Why the ack fails here: shared-cache SQLITE_LOCKED

SqliteProvider::new_in_memory connects with sqlite::memory:?cache=shared and max_connections(5).

Shared-cache mode replaces file-level locking with table-level read/write locks. A connection holding a read lock that tries to upgrade to a write lock while another connection still reads that table gets SQLITE_LOCKED (code 6) — reported by sqlx as database is deadlocked.

Two things follow from the SQLite shared-cache docs:

  1. PRAGMA busy_timeout = 60000 provides zero protection here. "If a required table lock cannot be obtained, the query fails and SQLITE_LOCKED is returned to the caller." No busy handler is invoked; recovering requires sqlite3_unlock_notify, which sqlx does not wire up.
  2. Writer starvation amplifies it. "After any attempt to obtain a write-lock on a table fails ... all attempts to open new transactions on the shared-cache fail until the current writer concludes." This is why fetch_work_item and fetch_orchestration_item also report SQLITE_LOCKED in the same window.

SQLite also documents shared-cache as obsolete: "Shared-cache mode is an obsolete feature. The use of shared-cache mode is discouraged."

Blast radius: shared-cache is used only by new_in_memory. File-backed deployments use WAL and get SQLITE_BUSY, which is covered by busy_timeout. So the specific SQLITE_LOCKED trigger is test-infrastructure-only — but Defect 1 applies to any provider that returns a retryable error from ack_work_item.

Reproduction

for i in $(seq 8); do cargo nt --test session_e2e_tests; done

Measured 2/8 failures in-suite. Also 1/5 in isolation:

for i in $(seq 5); do cargo nt -E 'test(test_multi_worker_heterogeneous_config)'; done

Isolation failing too is worth noting: the contention is intra-test (2 runtimes x (2 worker + 2 orchestration) dispatchers = 8 concurrent pollers against one shared cache with a 5-connection pool), not cross-test.

Failure log:

WARN duroxide::runtime::dispatchers::worker: worker: atomic ack failed,
abandoning work item instance=hetero-test execution_id=1 activity_id=3
worker_id=work-0-unconstrained-node
error=ack_work_item: error returned from database: (code: 6) database is deadlocked

Confirmed pre-existing: the assertion and the worker ack path are both unchanged at b6e0255 (origin/main).

Proposed fix

  1. Retry retryable ack failures in place with bounded exponential backoff before falling back to abandon, mirroring the existing fetch_work_item handling. Permanent errors keep abandoning immediately.
  2. Relax the test assertion to a + b >= 3, matching the documented at-least-once guarantee.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/runtime/dispatchers/worker.rs at handle_activity_outcome and compare its ack error handling with the fetch_work_item poll loop. Then inspect tests/session_e2e_tests.rs and run the named multi-worker test, including the provided repetition commands. Done means retryable ack failures back off before abandonment, permanent failures still abandon, and the test matches the documented at-least-once behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, sqlite
Domain
backend, databases, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.