Skip to content

Worker ACKs task event when persisting a successful reconciliation result fails #418

Description

@adirothbuilds

Summary

When reconciliation succeeds but persisting the resulting Task status fails, the worker still acknowledges the triggering event and proceeds to its next queue read.

This can leave the persisted Task status stale relative to the successfully reconciled runtime state until another reconciliation is triggered.

Current behavior

Verified against upstream base c5c1ac5:

c5c1ac5

The current sequence is:

TaskReconciler.Reconcile succeeds
→ processEvent calls UpdateTaskStatus
→ status persistence returns an error
→ processEvent returns that error
→ Worker.Run logs the processing error
→ Worker.Run still calls Ack for the same event
→ worker proceeds to the next queue read

If the status write did not commit, the reconciliation result is therefore not reflected in the persisted Task.

Characterization test

I added a test-only characterization in commit:

da235aa

adirothbuilds@da235aa

Branch:

https://github.com/adirothbuilds/ax/tree/test/controller-status-persistence-ack

The branch is exactly one commit ahead of the upstream base, and only internal/controller/worker_test.go changes. Production code is unchanged.

TestWorkerAcknowledgesEventWhenStatusPersistenceFails uses the real Worker and TaskReconciler, mock Substrate, MemoryStore, and test-only Store/Subscription decorators.

It deterministically verifies that:

  • reconciliation succeeds and computes Suspended
  • exactly one UpdateTaskStatus attempt is made
  • the injected failure occurs before touching the backing store
  • the same delivered event is still ACKed successfully
  • the persisted Task status remains Pending
  • the worker enters its next queue read without retrying reconciliation or status persistence

Channel barriers establish ordering; sleeps are not used as evidence. The focused test also passes under -race.

Redis-backed verification

I separately reproduced the same boundary with Docker Redis 7.4.11 using the real Redis Store and real XACK, while injecting only the status-persistence failure before delegation.

For the delivered event:

  • before ACK, XPENDING contained the exact event ID
  • XACK returned 1
  • afterward, the exact event ID was no longer present in the PEL
  • the persisted Task status remained Pending
  • no additional reconcile event was added to the stream
  • the worker proceeded to wait for another event

No automatic reconciliation occurred during the observation window. This does not imply that a later external Task event could not trigger another reconciliation.

The temporary Redis integration test was removed and is not part of the characterization commit.

Why this is distinct from related issues

Impact

This can leave the persisted Task status stale relative to the successfully reconciled runtime state until another reconciliation is triggered.

I could not find a periodic reconciliation, startup scan, status repair loop, or other deterministic recovery path for this exact sequence in the current implementation. A later external Task event can cause another reconciliation.

Design question

Worker.Run intentionally acknowledges processing failures so that a bad Task cannot wedge the queue.

Is that policy also intended to cover failure to persist the result of an otherwise successful reconciliation, or should this boundary have a different durability contract?

Activity

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions