Skip to content

A remote turn's post-commit writes land in the next crank's transaction and can be rolled back after the peer is answered #1072

Description

@sirtimid

Found reviewing #1021 (sirtimid/crank-rollback-integrity).

What

RemoteHandle.handleRemoteMessage and RemoteManager's incarnation change both do their durable work inside withStoreOutOfCrank(...), then do more store writes after the turn has been given back. Those later writes are in no turn, so they join whatever transaction the run loop has open by then — and the handoff makes that deterministic, not merely possible:

endOutOfCrank() resolves outOfCrankIdle from inside withStoreOutOfCrank's finally, which schedules the run loop's continuation before withStoreOutOfCrank's own promise resolves. The run loop then runs synchronously through startCrank()createCrankSavepoint('crank')createCrankSavepoint('delivery')await deliver(...). By the time the remote caller resumes, the delivery savepoint is open.

Failure scenario

  1. A peer sends redeemURL. The turn commits the c-list entry and highestReceivedSeq.
  2. Post-turn, #sendRemoteCommand persists setPendingMessage(remoteId, 5, …), setRemoteStartSeq and setRemoteNextSendSeq(…, 5), then puts redeemURLReply seq 5 on the wire. Those three writes are inside crank C's delivery savepoint.
  3. C's vat delivery throws; #processCrankResult rolls back to delivery. The DB reverts to nextSendSeq = 4 with no pending row for seq 5.
  4. In-memory #nextSendSeq is still 5, so nothing is noticed until restart. After restart the kernel reads 4, allocates 5 for a different message, and the peer — whose #highestReceivedSeq is 5 — drops it as a duplicate, logging only ignoring duplicate message seq=5.

Permanent, silent message loss.

RemoteManager is the same shape: finalizePeerRestart() mutates RAM and resolvePromises writes refcounts, promise resolutions and notify enqueues into crank C, which can discard them while the in-memory restart has already happened.

Pre-existing in kind. Worth filing because #1021's new comment now asserts the opposite: "Post-commit fan-out: in-memory state changes and run-queue mutations are not reversible by a savepoint, so they wait until the kv layer is durable." They do not wait, and they are not durable.

Suggested fix

Split #sendRemoteCommand so its three kernelStore writes are a synchronous unit, and run the post-turn completion in a second withStoreOutOfCrank (the wire send and the ACK timer stay outside it). Same for RemoteManager's fan-out. At minimum, correct the comment.

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