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).
Follow-ups from #6091 / PR #6171, which gave HA a per-protocol routing table (
getRoutingTable(ROUTING_PROTOCOL)) and made a follower'sgraphBatchLoadrefusal 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 aServerIsNotTheLeaderExceptionaround 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:portfor 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, andgetRoutingTablecan say so:nullis already the "no routing table" answer both callers handle:BoltNetworkExecutor.handleRoutefalls 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 assertingnulland moving the positive derive assertion to a unit test ofderiveRoutingAddress, which already exists.2. Only
graphBatchLoadnames the leader; every other leader-only refusal still names nothingGrpcErrorMapper.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:
graphBatchLoadis the only RPC inArcadeDbGrpcServicewith a leadership check at all. The others do not need one - they go throughRaftReplicatedDatabase, which raisesServerIsNotTheLeaderExceptionfor a schema change on a follower - but the two paths now report that same condition very differently:graphBatchLoadFAILED_PRECONDITIONServerIsNotTheLeaderExceptionABORTED(it is aNeedRetryException)ServerIsNotTheLeaderExceptionSince #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 - butgetLeaderAddress()returnsnullfor 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 aServerIsNotTheLeaderExceptionand the server has HA, attachLeaderRedirectProtocol.LEADER_GRPC_ADDRESS/LEADER_HTTP_ADDRESSthe same waynotTheLeader()does. Every leader-only refusal on the service becomes redirectable at once, andArcadeDbGrpcService.notTheLeadercollapses to choosing the status and the wording.GrpcErrorMapperis 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_PRECONDITIONsays "do not retry this as-is";ABORTEDsays "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 codearcadedb.ha.serverListgrew an object-formgrpc:field (host:{raft:2434,http:2480,bolt:7687,grpc:50051}) andarcadedb.grpc.portbecame a registeredGlobalConfiguration(so it now honours env vars and appears in the settings listing). The javadoc onRaftPeerAddressResolvercovers both, but the user-facing server-list reference lives in the docs repo, wherebolt:got a paragraph when #5002 added it. Thegrpc: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).