You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "peterxcli (via GitHub)" <gi...@apache.org> on 2026/09/20 16:07:09 UTC
[PR] Replace reflective OM follower routing with typed dispatch [ozone]
peterxcli opened a new pull request, #11289:
URL: https://github.com/apache/ozone/pull/11289
## What changes were proposed in this pull request?
Replace reflective OM follower routing with a concrete `OzoneManagerProtocolPB` implementation. Direct `submitRequest` calls remove argument-array parsing and reflective dispatch. A shared `RpcProxy` contract preserves connection lookup and lazy proxy creation, and consistency hints are applied before retries so fallback cannot change a request's hint.
## What is the link to the Apache JIRA
Pending Jira ID.
## How was this patch tested?
33 focused follower-routing and retry tests passed, along with checkstyle and RAT. The full `mvn clean install -DskipTests -DskipRecon` build also passed.
Generated-by: Codex (GPT-6)
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16529. Replace reflective OM follower routing with typed dispatch [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #11289:
URL: https://github.com/apache/ozone/pull/11289#discussion_r4068091295
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/HadoopRpcOMFollowerReadFailoverProxyProvider.java:
##########
@@ -148,34 +144,56 @@ public RetryPolicy getRetryPolicy(int maxFailovers) {
// for a few reasons
// 1. We want to ensure that the retry policy behavior remains the same when we use the leader proxy
// (when follower read is disabled or using write request)
- // 2. The FollowerInvocationHandler is also written so that the thrown exception is handled by the
+ // 2. The FollowerReadProxy is also written so that the thrown exception is handled by the
// OMFailoverProxyProviderbase's RetryPolicy
return leaderProxy.getRetryPolicy(maxFailovers);
}
/**
- * Parse the OM request from the request args.
- *
- * @return parsed OM request.
+ * Create a client that applies the default consistency hint once, before entering the retry loop.
*/
- private static OMRequest parseOMRequest(Object[] args) throws ServiceException {
- String error = null;
- if (args == null) {
- error = "args == null";
- } else if (args.length < 2) {
- error = "args.length == " + args.length + " < 2";
- } else if (args[1] == null) {
- error = "args[1] == null";
- } else if (!(args[1] instanceof OMRequest)) {
- error = "Non-OMRequest: " + args[1].getClass();
+ public OzoneManagerProtocolPB newProxy(int maxFailovers) {
+ OzoneManagerProtocolPB retryProxy = (OzoneManagerProtocolPB) RetryProxy.create(
+ OzoneManagerProtocolPB.class, this, getRetryPolicy(maxFailovers));
+ return new ReadConsistencyProxy(retryProxy);
+ }
Review Comment:
Let's document the multiple layer of proxy wrappings.
My understanding is now it's
- `ReadConsistencyProxy`
- `RetryProxy`
- `HadoopRpcOMFollowerReadFailoverProxyProvider`
- `HadoopRpcOMFailoverProxyProvider`
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16529. Replace reflective OM follower routing with typed dispatch [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #11289:
URL: https://github.com/apache/ozone/pull/11289#discussion_r4068053951
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/HadoopRpcOMFollowerReadFailoverProxyProvider.java:
##########
@@ -219,69 +237,47 @@ private synchronized OMProxyInfo<OzoneManagerProtocolPB> changeProxy(OMProxyInfo
}
/**
- * An InvocationHandler to handle incoming requests. This class's invoke
- * method contains the primary logic for redirecting to followers.
+ * A protocol implementation that redirects incoming requests to followers.
* <p>
* If follower reads are enabled, attempt to send read operations to the
* current proxy which can be either a leader or follower. If the current
* proxy's OM node fails, adjust the current proxy and return on the next one.
* <p>
* Write requests are always forwarded to the leader.
*/
- private class FollowerReadInvocationHandler implements RpcInvocationHandler {
+ private class FollowerReadProxy implements OzoneManagerProtocolPB, RpcProxy {
@Override
- public Object invoke(Object proxy, final Method method, final Object[] args)
- throws Throwable {
+ public OMResponse submitRequest(RpcController controller, OMRequest omRequest) throws ServiceException {
lastProxy = null;
- if (method.getDeclaringClass() == Object.class) {
- // If the method is not a OzoneManagerProtocolPB method (e.g. Object#toString()),
- // we should invoke the method on the current proxy
- return method.invoke(this, args);
- }
- OMRequest omRequest = parseOMRequest(args);
-
- // Apply default consistency hint once, before any routing decision.
- // In the future, we will support per-request hints which allows client (e.g. S3 clients)
- // to specify a custom request header (e.g. x-ozone-read-consistency) as a consistency hint
- // for read requests.
+ omRequest = applyReadConsistency(omRequest);
Review Comment:
Since we already have the `ReadConsistencyProxy`, do we still need the `applyReadConsistency` here? If this is not necessary, we can also put the `applyReadConsistency` under `ReadConsistencyProxy`.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/ha/HadoopRpcOMFollowerReadFailoverProxyProvider.java:
##########
@@ -148,34 +144,56 @@ public RetryPolicy getRetryPolicy(int maxFailovers) {
// for a few reasons
// 1. We want to ensure that the retry policy behavior remains the same when we use the leader proxy
// (when follower read is disabled or using write request)
- // 2. The FollowerInvocationHandler is also written so that the thrown exception is handled by the
+ // 2. The FollowerReadProxy is also written so that the thrown exception is handled by the
// OMFailoverProxyProviderbase's RetryPolicy
return leaderProxy.getRetryPolicy(maxFailovers);
}
/**
- * Parse the OM request from the request args.
- *
- * @return parsed OM request.
+ * Create a client that applies the default consistency hint once, before entering the retry loop.
*/
- private static OMRequest parseOMRequest(Object[] args) throws ServiceException {
- String error = null;
- if (args == null) {
- error = "args == null";
- } else if (args.length < 2) {
- error = "args.length == " + args.length + " < 2";
- } else if (args[1] == null) {
- error = "args[1] == null";
- } else if (!(args[1] instanceof OMRequest)) {
- error = "Non-OMRequest: " + args[1].getClass();
+ public OzoneManagerProtocolPB newProxy(int maxFailovers) {
+ OzoneManagerProtocolPB retryProxy = (OzoneManagerProtocolPB) RetryProxy.create(
+ OzoneManagerProtocolPB.class, this, getRetryPolicy(maxFailovers));
+ return new ReadConsistencyProxy(retryProxy);
+ }
Review Comment:
Let's document the nesting of the proxies
My understanding is now it's
- `ReadConsistencyProxy`
- `RetryProxy`
- `HadoopRpcOMFollowerReadFailoverProxyProvider`
- `HadoopRpcOMFailoverProxyProvider`
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ipc_/RpcProxy.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ipc_;
+
+import java.io.Closeable;
+import org.apache.hadoop.ipc_.Client.ConnectionId;
+
+/**
+ * Connection and lifecycle access for RPC proxies, including concrete protocol implementations.
+ */
+public interface RpcProxy extends Closeable {
Review Comment:
So `Closeable` and `getConnectionId` is extracted to `RpcProxy` so that it can be used as a proxy along with the `OzoneManagerProtocolPB` instead of as part of `InvocationHandler` (which does not implement the `OzoneManagerProtocolPB#submitRequest`).
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16529. Replace reflective OM follower routing with typed dispatch [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 merged PR #11289:
URL: https://github.com/apache/ozone/pull/11289
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16529. Replace reflective OM follower routing with typed dispatch [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #11289:
URL: https://github.com/apache/ozone/pull/11289#issuecomment-5788355687
@ivandika3 Thanks!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org