Skip to content

The Ratis-initiated snapshot install has neither guard the manual one has, so a follower can resync from its own address (or from the wrong peer) and report success #6202

Description

@lvca

Found while fixing #6191. ArcadeStateMachine pulls a database snapshot from the leader on two paths, and only one of them checks that the resolved address is not this node's own.

The guarded path

triggerSnapshotDownload() refuses twice, for the reason recorded in #6111: a node that "downloads" from itself copies its own incomplete databases back onto themselves, reports success, and lets resolveStaleSnapshotFloorAfterResync() durably record the marker index as applied - dropping the stale-read floor on state the node never had.

if (raftHAServer.isLeader()) { ...refuse... }
...
if (RaftHAServer.isSameHttpEndpoint(localHttpAddr, leaderHttpAddr)) { ...refuse... }   // ArcadeStateMachine:2589

The unguarded path

notifyInstallSnapshotFromLeader() - the Ratis-initiated install, i.e. the one that runs when a follower's log is behind the leader's compacted log - resolves the address and dials it with no check at all:

final String leaderHttpAddr = raftHAServer.getPeerHttpAddress(leaderId);
final String leaderHttpsAddr = raftHAServer.getPeerHttpsAddress(leaderId);
if (leaderHttpAddr == null)
  throw new RuntimeException("Cannot determine leader HTTP address for snapshot download");
...
reconciler.reconcileDatabasesFromLeader(leaderHttpAddr, leaderHttpsAddr, clusterToken);

getPeerHttpAddress is the same resolver #6191 was about: when a peer declares no http port in HA_SERVER_LIST, it derives the address as the peer's Raft host plus this node's own HTTP port. On a cluster whose nodes share a host - several nodes on one machine, a compose file, a developer laptop - every peer collapses onto this node's own endpoint. On a cluster with mixed ports it can just as easily name the wrong peer, and then the follower reconciles its databases from a node that is not the leader and may itself be behind.

Neither outcome reports an error. reconcileDatabasesFromLeader succeeds, the install is recorded, and the node returns to the ready set carrying whatever it copied.

Suggested fix

Apply the same two refusals triggerSnapshotDownload() already makes, through the same helper so they cannot drift: refuse when raftHAServer.isOwnHttpAddress(leaderHttpAddr) (added in #6195), and refuse when this node believes it is the leader. Ratis retries the install, so refusing leaves the follower out of the ready set and visibly behind, which is the honest state - the same disposition #6111 chose.

Worth considering in the same change: the address is only trustworthy if it is unambiguous. selectUnambiguousRouting (#6183) rejects an address two peers both resolve to, but it is applied to advertised routing tables only. The snapshot path is exactly where a wrong-but-plausible address does durable damage, so the ambiguity check belongs here more than it belongs in a routing table.

Two smaller things in the same method

  • The offload is CompletableFuture.supplyAsync(...) with no executor, i.e. the JDK common ForkJoinPool, against the rule in QueryEngineManager's class javadoc. The comment above it already concedes this and names the migration target (a dedicated executor sized by an arcadedb.haSnapshotInstallThreads knob). A snapshot install is a long download; the pool it starves is the one Gremlin and Polyglot user code runs on.
  • The single-flight CAS is documented as not serializing two concurrent installs when the CAS is lost - the loser proceeds anyway. Both pull from the same leader and SnapshotInstaller swaps atomically, so it is currently benign, but it is a race that will outlive the reasoning that makes it benign.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions