Skip to content

[BUG] MutableStore can discard pending writes or acknowledge unsynchronized changes #761

Description

@matt-ramotar

Overlapping operations for the same key in one MutableStore instance can leave a local write unsynchronized and remove the state needed to retry it.

Queue replacement can discard a newer write

Cleanup builds a replacement queue under the per-key mutex, then releases that mutex before installing the replacement. A newer write can enter the existing queue between those steps. See queue cleanup.

Possible sequence:

  1. Write A reaches the server successfully.
  2. A’s cleanup computes an empty replacement queue and releases the per-key mutex.
  3. Write B enters the existing queue and persists locally.
  4. A installs its replacement queue, discarding B.
  5. B’s synchronization attempt fails with No writes found for key=….

B receives an error, but this path bypasses failed-sync bookkeeping. The server can hold A while local storage holds B, with no pending queue entry or failure marker for B.

B may still exist locally. The failure is that Store has lost the state needed to synchronize it through eager recovery.

An older retry can acknowledge a newer failed write

An eager retry triggered by a read captures a local value before posting it. When the request succeeds, cleanup uses the completion time as its cutoff. See eager conflict resolution.

Possible sequence:

  1. An eager retry captures A and starts a slow server update.
  2. A newer write B persists locally, fails its server update, and records a failure marker.
  3. The older update of A succeeds.
  4. Its cleanup removes B, invokes B’s success callback using A’s result, and clears the failure marker.

B’s original update failed. Its callback can still report success even though B never reached the server.

Existing coverage

The two existing JVM concurrency tests passed. The concurrent test explicitly tolerates No writes found errors and checks for queue corruption. It does not establish synchronization correctness. See the tests.
#735 fixed the queue’s lock-polarity issue and left the logical same-key race outside its scope.

Acceptance criteria

I want the fix to preserve pending writes while keeping local writes responsive during slow network requests.

  • Cleanup cannot discard concurrently admitted writes.
  • Successful synchronization acknowledges only the writes represented by the value sent to the server.
  • Newer unsynchronized writes retain their pending state and failure bookkeeping.
  • Equal or out-of-order caller-provided timestamps cannot cause incorrect acknowledgments.
  • Newer writes can persist locally while an earlier server update is pending.
  • Deterministic tests cover both sequences and verify server values, callbacks, and recovery state.

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

    bugSomething isn't working

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions