ivandika3 commented on code in PR #10591:
URL: https://github.com/apache/ozone/pull/10591#discussion_r3481652375
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/GrpcOmTransport.java:
##########
@@ -174,6 +206,67 @@ public void start() throws IOException {
@Override
public OMResponse submitRequest(OMRequest payload) throws IOException {
+ if (useFollowerRead && OmUtils.shouldSendToFollower(payload)) {
+ return submitRequestWithFollowerRead(payload);
+ }
+ return submitRequestToLeader(addReadConsistencyHint(payload,
+ leaderReadConsistency));
+ }
+
+ private OMResponse submitRequestWithFollowerRead(OMRequest payload)
+ throws IOException {
+ OMRequest followerPayload = addReadConsistencyHint(payload,
+ followerReadConsistency);
+ int failedCount = 0;
+ for (int i = 0; useFollowerRead &&
+ i < omFailoverProxyProvider.getOMProxyMap().getNodeIds().size(); i++) {
+ String nodeId = getCurrentFollowerReadNodeId();
+ if (isCurrentLeaderNode(nodeId)) {
+ changeFollowerReadProxy(nodeId);
+ continue;
+ }
Review Comment:
Currently, we don't have this logic in the Hadoop RPC follower read since we
would like to allow read on leader. For parity purpose, let's remove this and
add them together if we want to introduce a follower only strategy.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/GrpcOmTransport.java:
##########
@@ -183,14 +276,7 @@ public OMResponse submitRequest(OMRequest payload) throws
IOException {
tryOtherHost = false;
expectedFailoverCount = globalFailoverCount.get();
try {
- InetAddress inetAddress = InetAddress.getLocalHost();
- Context.current()
- .withValue(GrpcClientConstants.CLIENT_IP_ADDRESS_CTX_KEY,
- inetAddress.getHostAddress())
- .withValue(GrpcClientConstants.CLIENT_HOSTNAME_CTX_KEY,
- inetAddress.getHostName())
- .run(() -> resp.set(clients.get(host.get())
- .submitRequest(payload)));
+ resp.set(submitRequestToHost(payload, host.get()));
Review Comment:
There are two AtomicReference set here and in the `submitRequestToHost`. Can
we use the return value of `submitRequestToHost` directly?
##########
hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/protocolPB/TestS3GrpcOmTransport.java:
##########
@@ -272,6 +282,214 @@ public void testGrpcFailoverExceedMaxMesgLen() throws
Exception {
assertThrows(Exception.class, () -> client.submitRequest(omRequest));
}
+ @Test
+ public void testFollowerReadSkipsKnownLeader() throws Exception {
Review Comment:
This test then can be changed so that if the initial OM is a follower, it
will not trigger failover.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
