HDDS-16737. Share OM follower read logic between the Hadoop RPC and gRPC clients by peterxcli · Pull Request #11418 · apache/ozone

peterxcli · 2026-10-06T09:19:46Z

What changes were proposed in this pull request?

The Hadoop RPC client and the gRPC client (used by the S3 Gateway) each have their own copy of the OM follower read logic. The Hadoop RPC one lives in HadoopRpcOMFollowerReadFailoverProxyProvider, while the gRPC one was added straight into GrpcOmTransport by HDDS-15492. So every follower read change, like the per-request read consistency in HDDS-15089, has to be made twice.

As suggested in #11252 (comment), this PR moves the shared parts into a base class:

FollowerReadFailoverProxyProviderBase keeps the follower read settings, decides whether a request may go to a follower, adds the default read consistency hint, and picks the OM node for follower reads (skipping the leader for LOCAL_LEASE).
HadoopRpcOMFollowerReadFailoverProxyProvider now extends it.
The gRPC follower read logic moves out of GrpcOmTransport into a new GrpcOMFollowerReadFailoverProxyProvider, which also extends it. GrpcOmTransport is left with just sending the requests.

The behavior stays the same. One detail: the Hadoop RPC provider now gets a node's proxy with createOMProxyIfNeeded(nodeId) each time it uses that node, the same way HadoopRpcOMFailoverProxyProvider#getProxy does for the leader. Before, it created the proxy only when moving to the next node. So the follower read index now starts at 0 instead of -1.

What is the link to the Apache JIRA

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

How was this patch tested?

New unit tests for the two new classes: TestFollowerReadFailoverProxyProviderBase and TestGrpcOMFollowerReadFailoverProxyProvider.
The existing follower read tests pass without changes: TestHadoopRpcOMFollowerReadFailoverProxyProvider, TestS3GrpcOmTransport, TestOzoneManagerHAFollowerReadWithAllRunning and TestOzoneManagerHAFollowerReadWithStoppedNodes.

…RPC clients

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

peterxcli · 2026-10-06T12:04:55Z

most of the addition are tests

ivandika3 · 2026-10-07T01:51:53Z

@echonesis Please help take a look as well since this concerns gRPC client.

ivandika3

Thanks @peterxcli for the patch, left some comments. There are some behavior change concerns.

echonesis

Thanks @peterxcli for the refactor.
Just leave some comments.

echonesis · 2026-10-07T18:11:26Z

+  public synchronized void changeInitialProxyForTest(String initialOmNodeId) {
+    currentIndex = getLeaderProxy().getOMProxyMap().indexOf(initialOmNodeId);


Could we retain the null check around indexOf(initialOmNodeId)?
An unknown node ID left the current selection unchanged. With this assignment, the returned Integer would be unboxed and throw an NPE.

I'd rather let it fail. The callers, tests and freon's follower affinity, pass node IDs from the same OM service. Silently ignoring a wrong ID would leave the client on a different OM without anyone noticing.

echonesis · 2026-10-07T18:13:33Z

+        for (int i = 0; i < getOMNodeCount(); i++) {
+          final String nodeId = selectFollowerReadNodeId(readConsistency);
+          if (nodeId == null) {
            break;
          }
+          final OMProxyInfo<OzoneManagerProtocolPB> current = leaderProxy.createOMProxyIfNeeded(nodeId);
          LOG.debug("Attempting to service submitRequest with cmdType {} using proxy {}",
              omRequest.getCmdType(), current.proxyInfo);
          try {


Would it be possible to add a regression test for that case?
It would be useful to verify that, after DNS refresh invalidates the current node’s cached proxy, the next read recreates it using the refreshed address and succeeds on the same node without moving to another OM.

DNS refresh belongs to the leader-based provider, and TestOMProxyInfoDnsRefresh#testProxyRebuildsAfterRefreshUsesNewAddress already covers it. The follower read provider just uses createOMProxyIfNeeded like the leader path. I'll reword the PR description so it doesn't sound like a new feature.

ivandika3

@peterxcli Thanks for the patch. LGTM +1.

chihsuan

Thanks for the patch! @peterxcli LGTM overall. The only thing before merging is the new base test failing on CI, see inline for a suggested fix.

chihsuan · 2026-10-25T03:21:53Z

+            disableFollowerRead();
+            break;
+          }
+          if (getLeaderNotReadyException(unwrapped) != null) {


Just curious, should this break to the leader on OMLeaderNotReadyException while Hadoop RPC rethrows, or could we unify them in a follow-up?

yes, let's me do this in follow-up, here I just moved the original code from https://github.com/apache/ozone/pull/10591/changes#diff-ca89512abffd117efea3925cd00f8e04b36b66ec09a66fff05e89b0b1bb9e584R234-R242

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

chihsuan · 2026-10-25T03:21:53Z

+  /** @return the inner proxy provider used for leader-based failover. */
+  protected abstract OMFailoverProxyProviderBase<OzoneManagerProtocolPB> getLeaderProxy();
+
+  public boolean isOmServiceSupportsFollowerRead() {


nit: Should we keep @VisibleForTesting here, as in the original Hadoop RPC provider?

they are just getter which doesnt affect anything

…a/TestFollowerReadFailoverProxyProviderBase.java Co-authored-by: Chi-Hsuan Huang <[email protected]>

peterxcli · 2026-10-25T11:43:07Z

Thanks @ivandika3 @chihsuan @echonesis!

* master: (114 commits) HDDS-17411. Show one initialization notice on the Namespace Usage page (apache#11737) HDDS-17375. Create new submodule ozone-integration-test-tools (apache#11754) HDDS-17452. Introduce ClientProtocol delegation for S3 Gateway (apache#11757) HDDS-16661. Validate unsigned S3 checksum trailer completion and framing (apache#11586) HDDS-17450. Speed up TestDirectoryDeletingService (apache#11765) HDDS-17308. ListObjectsV2 Incorrectly Filters CommonPrefixes When StartAfter Is Before the Matching Prefixes (apache#11715). Clamp Echo sleep time and response payload size. (apache#44) HDDS-17433. Add a freon command for positioned reads on shared input streams (apache#11741) HDDS-17239. Initialize usage counters on bucket create (apache#11742) HDDS-17357. Initialize Netty in background when Ozone client is created (apache#11726) HDDS-17426. Disable ozone.tracing.client.application-aware in acceptance tests (apache#11758) HDDS-16737. Share OM follower read logic between the Hadoop RPC and gRPC clients (apache#11418) HDDS-17446. Copy datanode container set in NodeStateMap.updateNode only once (apache#11746) HDDS-17454. Bump asm to 9.11.1 (apache#11750) HDDS-17453. Bump jackson to 2.21.8 (apache#11749) HDDS-17376. Scale repeated-defrag footprint tolerance with the number of OM tables (apache#11708) HDDS-17380. Fix deadlock when BackgroundContainerDataScanner shutdown during volume failure (apache#11729) HDDS-17251. List open multipart uploads in ListOpenFiles (apache#11739) HDDS-17361. S3 Gateway can set another user as the owner of a new key (apache#11707) HDDS-17238. Initialize refCount and usedNamespace on volume create (apache#11720) ...

HDDS-16737. Share OM follower read logic between the Hadoop RPC and g…

4d1516e

…RPC clients

Copilot AI balanced review requested due to automatic review settings October 6, 2026 09:19

Copilot AI reviewed Oct 6, 2026

peterxcli requested a review from ivandika3 October 6, 2026 12:05

peterxcli marked this pull request as ready for review October 6, 2026 12:05

ivandika3 reviewed Oct 7, 2026

View reviewed changes

echonesis reviewed Oct 7, 2026

View reviewed changes

Mark the node id of changeFollowerReadNodeId as Nonnull

c1999b7

peterxcli requested review from Russole, chihsuan, chungen0126, echonesis and ivandika3 and removed request for echonesis October 24, 2026 19:05

Merge branch 'master' into HDDS-16737

e7c1c38

ivandika3 approved these changes Oct 25, 2026

View reviewed changes

chihsuan approved these changes Oct 25, 2026

View reviewed changes

Update hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/h…

b18df92

…a/TestFollowerReadFailoverProxyProviderBase.java Co-authored-by: Chi-Hsuan Huang <[email protected]>

peterxcli merged commit b6d0181 into apache:master Oct 25, 2026
56 checks passed

peterxcli deleted the HDDS-16737 branch October 25, 2026 11:42

chihsuan mentioned this pull request Oct 26, 2026

HDDS-17448. gRPC OM client should retry a not-ready leader like the Hadoop RPC client #11760

Merged

		public synchronized void changeInitialProxyForTest(String initialOmNodeId) {
		currentIndex = getLeaderProxy().getOMProxyMap().indexOf(initialOmNodeId);

Conversation

peterxcli commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

peterxcli commented Oct 6, 2026

Uh oh!

ivandika3 commented Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivandika3 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

echonesis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivandika3 left a comment

Choose a reason for hiding this comment

Uh oh!

chihsuan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

peterxcli commented Oct 25, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

peterxcli commented Oct 6, 2026 •

edited

Loading

ivandika3 commented Oct 7, 2026 •

edited

Loading

ivandika3 left a comment •

edited

Loading