Separate coherent consensus details from KV-safe consensus queries

[eddyashton]

Context

Consensus state is exposed through many narrow accessors with inconsistent locking and no way to obtain a coherent multi-field snapshot.

The main Raft mutex is aft::State::lock. It serializes AFT message processing and protects the authoritative Raft state.

The KV side does not have a single mutex. Relevant locks include:

• Store::commit_lock, which serializes transaction commits;
• Store::version_lock, which protects version/term and replication bookkeeping;
• Store::maps_lock, which protects the collection of maps;
• each map's own state lock (untyped::Map::sl).

The call graph crosses this boundary in both directions. For example, KV commit code calls consensus (is_primary(), replicate()), while Raft commit/compaction calls back into the KV store and commit hooks.

For cross-component queries, the safe rule should be explicit: code holding any relevant KV lock must not acquire aft::State::lock. If a path must acquire both Raft and KV locks, the safe nesting order is Raft (aft::State::lock) first, then KV. Querying Raft in the opposite order can complete a KV -> Raft -> KV cycle.

replicate() is a mutating KV-to-Raft operation and is not solved by a query API split; its existing lock ordering should be audited and documented separately. The proposal here is to ensure ordinary observations cannot accidentally introduce additional reverse edges.

TSAN race/lock-order failures appear possible in this area and became highly reproducible on #8117, New RPCConnectionManager: Single unified manager, no ringbuffer, OpenSSL-owned sockets, although why that branch exposes them so consistently is not yet fully understood. #8181, Synchronize public AFT state queries (leader_id, leadership_state) is an initial attempt to make these reads safe, but currently treats a broad set of public consensus state as one synchronization domain.

Proposed separation

Remove the broad live-accessor API

Deliberately remove the current collection of live consensus accessors (primary(), is_primary(), is_backup(), get_view(), membership/retirement queries, and similar methods) from the externally visible consensus API.

Do not retain these as convenience wrappers. Keeping them would preserve the ability to compose independently timed reads, provide no coherent multi-field snapshot, and leave their locking contracts ambiguous.

For each existing accessor:

• current-state inspection should move to get_details();
• queries required while KV locks may be held should move to the narrow KV-query API;
• Raft-local helpers should become private and explicitly require aft::State::lock (or consensus-thread execution);
• historical queries and mutating operations should remain on separate purpose-specific surfaces.

General consensus inspection

Replace current-state inspection with a single:

get_details(options = {})

The default empty options should return the cheap fields. Options may request expensive data such as configurations or per-node acknowledgements. This function may take aft::State::lock once and copy the requested fields into one coherent snapshot.

Endpoint handlers and other ordinary callers should obtain one ConsensusDetails value and inspect that snapshot. Convenience predicates such as is_primary() may exist on the returned value, but not as separate live queries on consensus. Since ordinary endpoint execution does not hold the KV commit/map locks for the duration of the handler, taking the main Raft lock here is acceptable.

KV queries

Introduce a separate, deliberately narrow KV-query API for observations the KV must make while its own locks may be held, including commit, compaction, and snapshot paths.

These methods must never take aft::State::lock. They should read only state published under a dedicated kv_query_lock, making the lock-order contract explicit in both the API and implementation.

Candidate operations include the primary and view-history queries currently made from KV critical sections. The exact surface should be derived by auditing every KV-to-consensus call and distinguishing queries from mutating operations such as replicate().

Desired properties

• The existing broad live-accessor API is removed rather than supplemented.
• Endpoint callers obtain one coherent multi-field consensus snapshot through get_details().
• Expensive diagnostic fields are copied only when requested.
• KV critical sections cannot accidentally acquire aft::State::lock through a general accessor.
• The narrow published state and kv_query_lock are named for their actual purpose.
• The required Raft -> KV lock order is visible in the API structure rather than relying only on comments and call-site discipline.
• Consensus-internal code uses authoritative state under aft::State::lock or private helpers with that explicit contract.

[eddyashton]

Slightly briefer summary because I think the splurge above misses it:

The Raft public API is confused, and this leads to inconsistent locking discipline. It's unclear which methods are for external callers (should lock state in call), and which are internal helpers (expect lock to already be held. The worst are the "special" external callers - edges from the KV back into consensus, that need per-instance reasoning about what locks are held/should be held. I think the rational fix here is basically to nuke the public API - provide only a small handful of get_details()-like calls for external state access that definitely take a lock, make all the helpers private, and kill the back-edges. Turns out that last bit is surprisingly tractable by passing state from Raft into the KV, rather than expecting the KV to make callbacks (using the generic Consensus type that serves many uses).

The broad idea is mostly done, but some of the smallest and soundest-looking changes have caused rippling test failures that need independent fix. So this will probably land as a few PRs.