Skip to content

Follow-ups from #6091: a derived routing address can point a client back at the refusing node, and only graphBatchLoad names the leader #6183

Description

@lvca

Follow-ups from #6091 / PR #6171, which gave HA a per-protocol routing table (getRoutingTable(ROUTING_PROTOCOL)) and made a follower's graphBatchLoad refusal name a dialable gRPC address. None of these blocked that merge; they are recorded here so the reasoning is not re-derived.

Ordered by what I would fix first. Item 1 is a defect the new machine-readable redirect makes reachable; item 2 is a gap the issue explicitly asked about and left open; item 3 is docs.


1. A derived routing address can send a client straight back to the node that refused it

RaftHAServer.resolveRoutingAddress (ha-raft)

With no bolt: / grpc: field declared, a peer's client address is derived as that peer's Raft host + THIS node's port for the protocol. On a cluster where the nodes differ by port rather than by host - several nodes on one machine, a dev or test deployment - every peer derives to the same address, so the "leader" address a follower advertises is the follower's own endpoint.

Before #6171 the cost was confined to Bolt's ROUTE response, where a driver that dials a follower for a write gets an error and re-routes. #6171 raises it: the gRPC refusal puts that address on a trailer (arcadedb-leader-grpc-address) and the client rebuilds a ServerIsNotTheLeaderException around it, so a caller written to redirect automatically - the whole point of the trailer - can dial the same follower, be refused again, and loop.

The check is cheap and provable, because two distinct peers can never legitimately share a host:port for one protocol - they would be fighting over the socket. So a resolution that maps the leader and a follower to the same address is not a resolution, and getRoutingTable can say so:

// after resolving writer and readers, in getRoutingTable
if (readers.contains(writer))
  return null;   // the derive cannot distinguish these peers; advertise nothing rather than something wrong

null is already the "no routing table" answer both callers handle: BoltNetworkExecutor.handleRoute falls back to advertising this node as READ/ROUTE only (never writer), and the gRPC refusal falls back to naming the leader's HTTP address with the "use its gRPC port" caveat. Both degrade to what they did before, which is strictly better than a confidently wrong address.

Worth doing for both protocols in the shared resolver rather than for gRPC alone - a guard on one protocol only would be exactly the sort of divergence the generalisation in #6171 removed.

Note this changes what an in-process multi-node test observes, so Issue6091GrpcRoutingTableDerivedIT (which asserts the derived, same-host address today) has to be rewritten around the new contract - probably by asserting null and moving the positive derive assertion to a unit test of deriveRoutingAddress, which already exists.

2. Only graphBatchLoad names the leader; every other leader-only refusal still names nothing

GrpcErrorMapper.toStatusRuntimeException (grpcw), GrpcClientErrorMapper (grpc-client)

#6091 asked "whether other gRPC RPCs that refuse on a follower should use the same table". The answer found while implementing it: graphBatchLoad is the only RPC in ArcadeDbGrpcService with a leadership check at all. The others do not need one - they go through RaftReplicatedDatabase, which raises ServerIsNotTheLeaderException for a schema change on a follower - but the two paths now report that same condition very differently:

status exception class trailer leader address
graphBatchLoad FAILED_PRECONDITION ServerIsNotTheLeaderException on the trailers, gRPC + HTTP
everything else (engine-raised) ABORTED (it is a NeedRetryException) ServerIsNotTheLeaderException none

Since #6171 the client reconstructs the right type in both cases - before, the engine-raised one fell through to the status-code mapping and surfaced as a ConcurrentModificationException, which was simply wrong - but getLeaderAddress() returns null for everything except a batch load. A caller cannot tell "the leader is unknown, wait for the election" from "the leader is known and nobody bothered to tell you".

The fix belongs in GrpcErrorMapper, not in each RPC: when the cause is a ServerIsNotTheLeaderException and the server has HA, attach LeaderRedirectProtocol.LEADER_GRPC_ADDRESS / LEADER_HTTP_ADDRESS the same way notTheLeader() does. Every leader-only refusal on the service becomes redirectable at once, and ArcadeDbGrpcService.notTheLeader collapses to choosing the status and the wording. GrpcErrorMapper is currently static and holds no server reference, which is the one design question to settle.

While there: the status codes should probably agree with each other too. FAILED_PRECONDITION says "do not retry this as-is"; ABORTED says "retry". For "you are talking to the wrong node" the honest answer is the first, with the address saying where.

3. The grpc: field is undocumented outside the code

arcadedb.ha.serverList grew an object-form grpc: field (host:{raft:2434,http:2480,bolt:7687,grpc:50051}) and arcadedb.grpc.port became a registered GlobalConfiguration (so it now honours env vars and appears in the settings listing). The javadoc on RaftPeerAddressResolver covers both, but the user-facing server-list reference lives in the docs repo, where bolt: got a paragraph when #5002 added it. The grpc: field wants the same, including the sentence that matters operationally: declare it whenever your nodes do not all listen on the same gRPC port, because the fallback silently assumes they do (see item 1).

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions