szetszwo commented on code in PR #10474:
URL: https://github.com/apache/ozone/pull/10474#discussion_r3383934557
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java:
##########
@@ -224,11 +224,7 @@ public SCMStateMachine getSCMStateMachine() {
@Override
public void registerStateMachineHandler(final RequestType handlerType,
final Object handler) {
Review Comment:
Let's change the handler type to ScmInvoker<?> and remove the casting.
```diff
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServer.java
@@ -37,7 +37,7 @@ public interface SCMRatisServer {
void start() throws IOException;
- void registerStateMachineHandler(RequestType handlerType, Object handler);
+ void registerStateMachineHandler(RequestType handlerType, ScmInvoker<?>
handler);
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManagerStub.java:
##########
@@ -225,10 +218,10 @@ public boolean triggerSnapshot() throws IOException {
private Message process(final SCMRatisRequest request) throws Exception {
final ScmInvoker<?> invoker = invokers.get(request.getType());
- if (invoker != null) {
- return invoker.invokeLocal(request.getOperation(),
request.getArguments());
+ if (invoker == null) {
+ throw new IOException("No handler found for request type " +
request.getType());
Review Comment:
Use `Objects.requireNonNull(invoker, "invoker == null");`
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMStateMachine.java:
##########
@@ -201,28 +192,10 @@ public CompletableFuture<Message> applyTransaction(
private Message process(final SCMRatisRequest request) throws Exception {
final ScmInvoker<?> invoker = invokers.get(request.getType());
- if (invoker != null) {
- return invoker.invokeLocal(request.getOperation(),
request.getArguments());
- }
- return process(request, handlers.get(request.getType()));
- }
-
- public static Message process(final SCMRatisRequest request, Object handler)
throws Exception {
- try {
- if (handler == null) {
- throw new IOException("No handler found for request type " +
- request.getType());
- }
-
- final Method method =
handler.getClass().getMethod(request.getOperation(),
request.getParameterTypes());
- final Object result = method.invoke(handler, request.getArguments());
- return SCMRatisResponse.encode(result, method.getReturnType());
- } catch (NoSuchMethodException | SecurityException ex) {
- throw new InvalidProtocolBufferException(ex.getMessage());
- } catch (InvocationTargetException e) {
- final Exception targetEx = (Exception) e.getTargetException();
- throw targetEx != null ? targetEx : e;
+ if (invoker == null) {
+ throw new IOException("No handler found for request type " +
request.getType());
}
Review Comment:
Use
```java
Objects.requireNonNull(invoker, "invoker == null");
```
--
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]
