HDDS-14922. Define ScmInvoker#invokeRatisServer to proxy DeletedBlockLogStateManager without reflection

[Russole]

What changes were proposed in this pull request?

• Add invokeRatisServer(..) to ScmInvoker to centralize Ratis server invocation logic
• Move Ratis server invocation from SCMHAInvocationHandler to ScmInvoker
• Update SCMHAInvocationHandler to delegate DIRECT invocation to ScmInvoker
• Add fallback to the original implementation when ScmInvoker is not available (for backward compatibility)
• Implement the new invocation path for DeletedBlockLogStateManager

Follow-up

• Complete the refactoring for DeletedBlockLogStateManager first; other SCM handlers will be updated in follow-up patches
• Apply the same invocation pattern to invokeRatisClient(..) in future patches

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-14922

How was this patch tested?

• All CI checks passed
  • https://github.com/Russole/ozone/actions/runs/23797561288

[Russole]

Hi @szetszwo, could you please review this PR when you have time? Thank you!

[szetszwo]

@Russole , thanks for working on this! We should not use Proxy.newProxyInstance(..) anymore; see https://issues.apache.org/jira/secure/attachment/13081517/10013_review.patch

[szetszwo]

This change does not help since it is after the proxy. We should do it before the proxy.

[Russole]

Thanks @szetszwo for the review. Updated the implementation to handle overloaded @Replicate methods and pass parameter types to invokeRatisServer, ensuring correct method resolution during Ratis invocation.
Without this, incorrect overload resolution could lead to null protobuf arguments being serialized, causing a NullPointerException in CI tests (e.g. TestScmDataDistributionFinalization).
Also updated the implementation based on the review comments.

[szetszwo]

... Without this, incorrect overload resolution could lead to null ...

@Russole , Good catch on the bug!

Please see the comments inlined and also https://issues.apache.org/jira/secure/attachment/13081552/10013_review2.patch

[szetszwo]

The invoker field is always null. We should remove it.

[szetszwo]

We don't need the common method anymore.

  default <T> T getProxyHandler(ScmInvoker<T> invoker) {
    registerStateMachineHandler(invoker.getType(), invoker.getImpl());
    return invoker.getProxy();
  }

  default <T> T getProxyHandler(RequestType type, Class<T> intf, T impl) {
    final SCMHAInvocationHandler invocationHandler =
        new SCMHAInvocationHandler(type, impl, this);
    return intf.cast(Proxy.newProxyInstance(getClass().getClassLoader(),
        new Class<?>[] {intf}, invocationHandler));
  }

[szetszwo]

Let's add an enum instead:

  enum ReplicateMethod implements NameAndParameterTypes {
    addTransactionsToDB(new Class<?>[] {ArrayList.class, DeletedBlocksTransactionSummary.class}),
    removeTransactionsFromDB(new Class<?>[] {ArrayList.class, DeletedBlocksTransactionSummary.class}),
    ;

    private final Class<?>[][] parameterTypes;

    ReplicateMethod(Class<?>[] parameterTypes) {
      final Class<?>[][] types = new Class<?>[parameterTypes.length + 1][];
      for(int i = 0; i <= parameterTypes.length; ++i) {
        types[i] = Arrays.copyOf(parameterTypes, i);
      }
      this.parameterTypes = types;
    }

    @Override
    public String getName() {
      return name();
    }

    @Override
    public Class<?>[] getParameterTypes(int numArgs) {
      return parameterTypes[numArgs];
    }
  }

[szetszwo]

Let's add an interface:

  interface NameAndParameterTypes {
    String getName();

    Class<?>[] getParameterTypes(int numArgs);
  }

  Object invokeRatisServer(NameAndParameterTypes method, Object[] args) throws SCMException {
    try {
      final SCMRatisRequest request = SCMRatisRequest.of(
          getType(), method.getName(), method.getParameterTypes(args.length), args);

[szetszwo]

+1 the change looks good.

[adoroszlai]

Thanks @Russole for the patch, @szetszwo for the review.