Skip to content

cluster: a runner that lost its shard lock can still write replies and messages for that shard #10058

Open

Description

@heyimcarlos

Base: effect 4.0.6 (effect/cluster, SqlMessageStorage + SqlRunnerStorage on PostgreSQL advisory locks). main has the same code.

What happens. Shard ownership is a session advisory lock. A runner checks it every shardLockRefreshInterval.
Between two checks, the runner can lose the lock (its lock session ended, or it was paused) while another runner
acquires the shard, runs resetShards, and starts the same entities. The old runner does not know yet. Its handlers
finish and call saveReply, saveRequest, saveEnvelope, clearReplies and clearAddress. None of these check
who owns the shard, so the writes succeed.

Results:

  • A persisted request gets two replies, or the old owner's reply wins over the new owner's.
  • A clearReplies or clearAddress from the old owner deletes what the new owner just wrote.
  • With a duplicate primary key, the write is deduplicated with no error, so nobody sees it.

Repro (two runners believe they own one shard):

const cluster = yield* make({ backend: "pg", entities: HoldEntityLayer }) // persisted RPC that waits on a gate
const [stale] = yield* cluster.start(1)
const reply = yield* client("held").Hold(new Request({ id: "fenced" })).pipe(Effect.forkChild)
// handler has started on `stale`
yield* cluster.freeze(stale)              // stale stops refreshing
yield* cluster.dropLockSessions           // pg_terminate_backend on its lock session
const [owner] = yield* cluster.start(1)   // owner acquires the shard and re-runs the request
assert(stale.sharding.hasShardId(shard))  // stale still believes it owns it
openGate(stale)                           // stale's handler finishes first
// 4.0.6: stale's reply is stored and returned to the caller.
// Expected: stale's saveReply fails; only owner's reply is stored.

Expected. Each shard acquisition gets an epoch. Every write on behalf of a shard carries the epoch the writer
acquired with, and storage rejects it in the same transaction if the shard's epoch has moved on. The rejection is a
typed error, logged at Error, and the entity on the stale runner stops.

Proposed fix (implemented on a fork, PR available).

  • SqlMessageStorage migration 0004_shard_epochs adds <prefix>_shards (shard_id, epoch).
  • resetShards raises the epoch of each shard it resets and returns a ShardFence per shard.
  • MessageStorage.CurrentShardFence carries the fence to the write. saveRequest, saveEnvelope, saveReply,
    clearReplies and clearAddress read the shard's epoch with a shared row lock (FOR SHARE, LOCK IN SHARE MODE,
    HOLDLOCK) inside the write's transaction, and fail with ShardFenced if it differs. resetShards takes the
    exclusive lock on the same row, so a stale write either commits before the new owner's reset or fails.
  • Writes by a client (sending to a shard it does not own) carry no fence and are not checked.
  • An entity whose reply is fenced is stopped. Sharding releases a shard whose reset fails.

Runnable repro (real runner processes, fails on 4.0.6 and on main at 837e906): https://github.com/heyimcarlos/effect-cluster-repro, issue 3 in the README table.

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