[peterxcli]

What changes were proposed in this pull request?

When the OM leader is not ready yet, for example right after a leader change, the Hadoop RPC client keeps retrying that OM with a growing wait. The gRPC client used by the S3 Gateway doesn't:

• The OM retry policy only recognizes OMLeaderNotReadyException when it gets a ServiceException, but GrpcOmTransport#unwrapException hands it the OM exception itself. So the gRPC client treats a not-ready leader like any other failure and fails over to the next OM without waiting.
• Only the leader path has a retry loop. When a follower read reaches a not-ready leader, it falls back to the leader path and sends another request right away.

With this change:

• unwrapException returns a ServiceException with the RemoteException cause for errors from the OM, the same shape the Hadoop RPC client passes to the retry policy. This also drops the reflection that rebuilt the OM exception there.
• The retry loop moves from submitRequestToLeader up to GrpcOmTransport#submitRequest, so it covers both follower read and the leader path, like RetryProxy does for the Hadoop RPC client.
• The gRPC follower read throws OMLeaderNotReadyException to that loop instead of falling back to the leader, the same as HadoopRpcOMFollowerReadFailoverProxyProvider.

A follower read error that can't fail over, like RESOURCE_EXHAUSTED, now ends as an OMException, the same as on the leader path, instead of a raw StatusRuntimeException.

The client still reads the exception class and message from the status description. Replacing that with a structured error from the OM is HDDS-17462.

What is the link to the Apache JIRA

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

How was this patch tested?

New tests in TestS3GrpcOmTransport, with an OM leader that isn't ready for its first two requests:

• a leader request is retried on the same OM until it succeeds, and the other OM never sees it;
• a follower read is retried the same way, and every retry is still a follower read.

TestGrpcOMFollowerReadFailoverProxyProvider now expects OMLeaderNotReadyException to be thrown to the caller. All three tests fail without the fix.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

[chihsuan]

Thanks for the follow-up on #11418! @peterxcli LGTM

[ivandika3]

Thanks @peterxcli , LGTM +1. Left some minor comments.

[ivandika3]

Nit: Let's take this opportunity to cleanup this tryOtherHost condition. It's currently not used. We can also rename it to shouldRetry.

Also the

        if (!tryOtherHost) {
          throw new OMException(resultCode);
        }

is not needed since outside the loop we already throw OMException.

Additionally, shouldRetry actually does a failover so shouldRetry might not be a good method name.

I'm OK if you defer this to a follow up ticket. The GrpcOmTransport is readability is pretty poor IMO so we might need to have a wider cleanup / refactor. To name a few

• Separate the concern for exception handling by using some kind of gRPC StatusRuntimeException to Hadoop RemoteException adapter.
• Rename host to currentHost (or leaderHost if it's supposed to be pointed to the leader).
• Separate the gRPC channel building to a separate method
• Do a compatibility test on the gRPC exception message so that it can be parsed by the client.

[Russole]

Thanks @peterxcli for the PR. LGTM!

[adoroszlai]

Thanks @peterxcli for the patch, @chihsuan, @ivandika3, @Russole for the review.