cluster: the local node's writer state is never set, so IsPrimaryWriter() with a failover manager is always false on the primary itself

[xe-nvdk]

Found by the #807 review (pre-existing, not changed by that fix).

Coordinator.IsPrimaryWriter() returns c.localNode.IsPrimaryWriter() when a writer-failover manager is configured (internal/cluster/coordinator.go, around the IsPrimaryWriter method). Nothing in production ever sets c.localNode's writer state: onWriterPromoted mutates the registry's clones and re-registers them, and the #807 nodeFromRaftInfo carries the snapshot's writer state into the registry copy only. c.localNode is an identity pointer that Registry.Local() returns and that Register never replaces, so the primary writer's own gate reads a writer state that stays empty.

Effect: every caller of IsPrimaryWriter() on the failover-manager path (writer-only schedulers such as retention and CQ, the compaction gate) sees "not primary" on the node the FSM promoted. Shared-storage mode is unaffected (it keys off Raft leadership and role).

Fix shape: have onWriterPromoted (and the restore seeding) also update c.localNode when the promoted or demoted ID is the local node, or make IsPrimaryWriter read the registry entry for the local ID instead of the identity pointer. Add a test that promotes the local node through the FSM and asserts IsPrimaryWriter().

[xe-nvdk]

Reproduced on a live 3-writer + 1-reader cluster (deploy/docker-compose/enterprise-local, image built from c895b869), and the root cause is upstream of what this issue describes.

What I ran. Same cluster, same retention policy, same node; only cluster.failover_enabled differs. POST /api/v1/retention/:id/execute with dry_run:false reads IsPrimaryWriter() directly, so it is a clean probe of the gate.

cluster.failover_enabled	dry run	real run
true	200	503 retention rejected: node role "writer" is not primary writer
false	200	200

Root cause: with writer failover enabled, no writer is ever promoted, so there is no primary at all. /api/v1/cluster/nodes reports writer_state empty on all three writers, indefinitely. WriterFailoverManager.checkPrimaryHealth is the only path to a promotion, and it bootstrap-deadlocks:

primary := m.cfg.Registry.GetPrimaryWriter()
if primary == nil {
    writers := m.cfg.Registry.GetWriters()
    if len(writers) > 0 && m.primaryID != "" {   // m.primaryID is "" until a primary exists
        ...triggerFailoverLocked()
    }
    return
}
m.primaryID = primary.ID                          // the only place it is ever set

m.primaryID is only assigned when a primary already exists, and HandleWriterUnhealthy returns early unless node.GetWriterState() == WriterStatePrimary. PromoteWriter has exactly one production caller, inside triggerFailoverLocked, so the first promotion can never happen. Nothing else in the tree ever sets WriterStatePrimary except the promotion callback at coordinator.go:2389.

The compactor manager has the path this one is missing: CompactorFailoverManager.tryInitialAssignment assigns an initial lease when none is active, and the same run logs Assigning initial compactor lease node_id=arc-writer1.

Effect. In Pattern 1 with failover_enabled=true, IsPrimaryWriter() is false on every node, so the retention and continuous-query schedulers never run a tick and the retention, CQ and delete endpoints refuse every non-dry-run call, cluster-wide. Retention silently never running is the expensive one. Turning failover off restores all of it, because IsPrimaryWriter then short-circuits to node.Role == RoleWriter.

This issue's original finding still stands, as a second defect behind the first. Once an initial election exists, the promotion callback mutates the registry's *Node, which is no longer the same object as c.localNode after onRaftNodeAdded replaces the registry entry, while the gate reads c.localNode via GetLocalNode(). So a fix needs both halves: an initial election, and a promotion that lands where the gate looks. Retitling suggested: the missing initial election is the headline.

Repro scripts: scratchpad/850/e2e850.sh plus override850-failover.yml in the session worktree.

[xe-nvdk]

Follow-up on whether the load-balancer architecture supersedes this. It does not, and here is the evidence either way.

The LB change is real, and it is Pattern 2 only. SharedStorageMode (PRs #463 / #464) is documented in config.go as "multiple RoleWriter nodes sharing a single object-storage backend behind a load balancer", and when it is on the coordinator logs Writer failover suppressed: cluster.shared_storage_mode=true (Pattern 2 multi-writer; LB handles writer-crash failover) and IsPrimaryWriter() switches to "is Raft leader AND RoleWriter". No promotion is involved, so Pattern 2 is fine by construction, and TestIsPrimaryWriter_SharedStorageMode_* pins it.

Pattern 1 was not retired by that change. cluster.shared_storage_mode still defaults to false; the coordinator still builds the writer-failover manager whenever failover_enabled && !shared_storage_mode; the clustering docs still promise "Automatic promotion: a reader (acting as standby) is promoted to writer via Raft consensus" with "Recovery time: less than 30 seconds"; and the Enterprise Helm chart ships cluster.failover.enabled: true by default with a documented Pattern 1 example of one writer plus three readers where "readers double as failover pool".

Re-ran the repro in exactly that documented topology (1 writer + 3 readers, failover_enabled=true, local storage). Same result as the 3-writer run: every node reports an empty writer_state, and the single writer refuses its own retention execute with retention rejected: node role "writer" is not primary writer.

Pattern 1's promotion never worked; nothing removed it. internal/cluster/writer_failover.go has only ever been touched by the PR that introduced it (#179) and a repo-wide gofmt, so there was never an initial-election path to lose. The compactor manager, written later in #390, has the piece this one lacks (tryInitialAssignment).

Why it stayed hidden. #179 merged with its own test plan item "Integration test with multi-node cluster (manual)" left unchecked, and the unit tests set the promoted state directly on the local node (c.localNode.SetWriterState(...)). They prove the gate reads a promoted state correctly; nothing ever asserted that a promotion produces one.

So the decision is a product one, and either answer requires a change before the next release, because the chart default silently disables retention and CQ today:

1. Pattern 1 + writer failover stays supported: fix the initial election and the pointer divergence.
2. The LB architecture supersedes it: retire it honestly in the docs and the chart default, and make failover_enabled on a non-shared-storage cluster refuse at startup instead of quietly disabling the singleton schedulers.