Skip to content

HDDS-14922. Define invokeRatisServer or invokeRatisClient methods#10013

Open
Russole wants to merge 2 commits intoapache:masterfrom
Russole:HDDS-14922
Open

HDDS-14922. Define invokeRatisServer or invokeRatisClient methods#10013
Russole wants to merge 2 commits intoapache:masterfrom
Russole:HDDS-14922

Conversation

@Russole
Copy link
Copy Markdown
Contributor

@Russole Russole commented Mar 31, 2026

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?

@Russole
Copy link
Copy Markdown
Contributor Author

Russole commented Mar 31, 2026

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

Copy link
Copy Markdown
Contributor

@szetszwo szetszwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Comment on lines +114 to +117
if (invoker != null) {
return invoker.invokeRatisServer(
method.getName(), method.getParameterTypes(), args);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants