szetszwo commented on code in PR #1382:
URL: https://github.com/apache/ratis/pull/1382#discussion_r3046956757
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -868,20 +895,18 @@ private CompletableFuture<RaftClientReply>
appendTransaction(
} catch (StateMachineException e) {
// the StateMachineException is thrown by the SM in the preAppend
stage.
// Return the exception in a RaftClientReply.
- RaftClientReply exceptionReply = newExceptionReply(request, e);
- cacheEntry.failWithReply(exceptionReply);
+ final RaftClientReply exceptionReply = newExceptionReply(request, e);
// leader will step down here
if (e.leaderShouldStepDown() && getInfo().isLeader()) {
leaderState.submitStepDownEvent(StepDownReason.STATE_MACHINE_EXCEPTION);
}
- return CompletableFuture.completedFuture(exceptionReply);
+ return failWithReply(exceptionReply, cacheEntry, context);
Review Comment:
StateMachineException is from StateMachine. We should not cancel the
transaction.
##########
ratis-server/src/main/java/org/apache/ratis/server/impl/RaftServerImpl.java:
##########
@@ -1014,19 +1031,26 @@ private CompletableFuture<RaftClientReply>
writeAsyncImpl(RaftClientRequest requ
// return the cached future.
return cacheEntry.getReplyFuture();
}
- // TODO: this client request will not be added to pending requests until
- // later which means that any failure in between will leave partial state
in
- // the state machine. We should call cancelTransaction() for failed
requests
+ // This request will be added to pending requests later in
appendTransaction.
+ // Any failure in between must invoke cancelTransaction.
final TransactionContextImpl context = (TransactionContextImpl)
stateMachine.startTransaction(
filterDataStreamRaftClientRequest(request));
if (context.getException() != null) {
- final StateMachineException e = new StateMachineException(getMemberId(),
context.getException());
+ final Exception exception = context.getException();
+ final StateMachineException e = new StateMachineException(getMemberId(),
exception);
final RaftClientReply exceptionReply = newExceptionReply(request, e);
- cacheEntry.failWithReply(exceptionReply);
- return CompletableFuture.completedFuture(exceptionReply);
+ return failWithReply(exceptionReply, cacheEntry, context);
}
- return appendTransaction(request, context, cacheEntry);
+ try {
+ return appendTransaction(request, context, cacheEntry);
+ } catch (IOException ioe) {
+ cancelTransaction(context, ioe);
+ throw ioe;
+ } catch (RuntimeException re) {
+ cancelTransaction(context, re);
+ throw re;
+ }
Review Comment:
Just catch (Exception e) instead of repeating the code.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
