Cyrill commented on code in PR #6408:
URL: https://github.com/apache/ignite-3/pull/6408#discussion_r2282998742
##########
modules/raft/src/main/java/org/apache/ignite/raft/jraft/rpc/RpcRequestProcessor.java:
##########
@@ -52,21 +53,16 @@ public void handleRequest(final RpcContext rpcCtx, final T
request) {
if (msg != null) {
rpcCtx.sendResponse(msg);
}
+ } catch (NotLeaderException t) {
+ // It is ok if we lost leadership while a request to us was in
flight, there is no need to clutter up the log.
+ LOG.debug("handleRequest {} failed", t, request);
+ rpcCtx.sendResponse(RaftRpcFactory.DEFAULT
+ .newResponse(t.leaderId(), msgFactory, RaftError.EPERM,
t.getMessage()));
+ } catch (Throwable t) {
+ LOG.error("handleRequest {} failed", t, request);
+ rpcCtx.sendResponse(RaftRpcFactory.DEFAULT
+ .newResponse(msgFactory, RaftError.UNKNOWN.getNumber(),
"handleRequest internal error"));
}
- catch (final Throwable t) {
- if (isIgnorable(t)) {
- LOG.debug("handleRequest {} failed", t, request);
- } else {
- LOG.error("handleRequest {} failed", t, request);
- }
- rpcCtx.sendResponse(RaftRpcFactory.DEFAULT //
Review Comment:
Correct.
The return code -1 translates to RaftError.UNKNOWN. What we had were
constant retries until sendWithRetry failed with a timeout.
`UNKNOWN` handler looks like:
```
NetworkMessage request = retryContext.request();
Peer newTargetPeer;
// If changing peers or requesting a leader and something is
not found
// probably target peer is doing rebalancing, try another
peer.
if (request instanceof GetLeaderRequest || request
instanceof ChangePeersAndLearnersAsyncRequest) {
newTargetPeer = randomNode(retryContext);
} else {
newTargetPeer = retryContext.targetPeer();
}
scheduleRetry(fut, retryContext.nextAttempt(newTargetPeer,
getShortReasonMessage(retryContext, error, resp)));
```
Since the request I faced this issue with was ResetLearnersRequest, the code
always falls into the else branch without changing the target node.
And I don't think adding one more request here is a good design.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
