ivandika3 commented on code in PR #8542:
URL: https://github.com/apache/ozone/pull/8542#discussion_r3464854300
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/StateContext.java:
##########
@@ -766,6 +764,8 @@ public SCMCommand<?> getNextCommand() {
return command;
}
+ updateCommandStatus(command.getId(),
+ status -> status.setStatus(Status.FAILED));
LOG.warn("Detect and drop a SCMCommand {} from stale leader SCM," +
" stale term {}, latest term {}.",
command, command.getTerm(), currentTerm);
Review Comment:
Nit: Just a code smell improvement, I think it's better to have the
`command.getTerm() != currentTerm` case earlier to fail fast since previously
we already have `command == null`.
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/commandhandler/CommandDispatcher.java:
##########
@@ -82,26 +81,26 @@ public CommandHandler getDeleteBlocksCommandHandler() {
return handlerMap.get(Type.deleteBlocksCommand);
}
- public ClosePipelineCommandHandler getClosePipelineCommandHandler() {
- return (ClosePipelineCommandHandler)
handlerMap.get(Type.closePipelineCommand);
- }
-
/**
* Dispatch the command to the correct handler.
*
* @param command - SCM Command.
*/
public void handle(SCMCommand<?> command) {
- Objects.requireNonNull(command, "command == null");
+ Preconditions.checkNotNull(command);
CommandHandler handler = handlerMap.get(command.getType());
if (handler != null) {
commandHandlerMetrics.increaseCommandCount(command.getType());
try {
handler.handle(command, container, context, connectionManager);
} catch (Exception ex) {
+ context.updateCommandStatus(command.getId(),
+ status -> status.setStatus(CommandStatus.Status.FAILED));
LOG.error("Exception while handle command, ", ex);
}
} else {
+ context.updateCommandStatus(command.getId(),
+ status -> status.setStatus(CommandStatus.Status.FAILED));
Review Comment:
This is a general review for most of the command, is it easier to throw
exception and let the `updateCommandStatus` to be done in the catch block?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
