From a6bea06fc0710fbf0c3a6865f254ec23b476fe63 Mon Sep 17 00:00:00 2001 From: Sammi Chen Date: Mon, 25 Nov 2024 18:01:38 +0800 Subject: [PATCH] remove SHORT_CIRCUIT replication type --- .../hadoop/hdds/scm/XceiverClientCreator.java | 9 +- .../hadoop/hdds/scm/XceiverClientManager.java | 12 +- .../hadoop/hdds/client/ReplicationConfig.java | 3 - .../hadoop/hdds/client/ReplicationType.java | 7 +- .../client/ShortCircuitReplicationConfig.java | 129 ------------------ .../src/main/proto/hdds.proto | 1 - .../hdds/scm/TestXceiverClientManagerSC.java | 3 +- 7 files changed, 9 insertions(+), 155 deletions(-) delete mode 100644 hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ShortCircuitReplicationConfig.java diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientCreator.java b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientCreator.java index 4d901e46565..d845a835db1 100644 --- a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientCreator.java +++ b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientCreator.java @@ -85,14 +85,15 @@ protected XceiverClientSpi newClient(Pipeline pipeline, DatanodeDetails dn) thro client = XceiverClientRatis.newXceiverClientRatis(pipeline, conf, trustManager, errorInjector); break; case STAND_ALONE: - client = new XceiverClientGrpc(pipeline, conf, trustManager); + if (dn != null) { + client = new XceiverClientShortCircuit(pipeline, conf, dn); + } else { + client = new XceiverClientGrpc(pipeline, conf, trustManager); + } break; case EC: client = new ECXceiverClientGrpc(pipeline, conf, trustManager); break; - case SHORT_CIRCUIT: - client = new XceiverClientShortCircuit(pipeline, conf, dn); - break; case CHAINED: default: throw new IOException("not implemented " + pipeline.getType()); diff --git a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java index 802b3449575..30c8c8e766f 100644 --- a/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java +++ b/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java @@ -200,17 +200,9 @@ public void releaseClient(XceiverClientSpi client, boolean invalidateClient, protected XceiverClientSpi getClient(Pipeline pipeline, boolean topologyAware, boolean allowShortCircuit) throws IOException { try { - // create different client different pipeline node based on - // network topology + // create different client different pipeline node based on network topology String key = getPipelineCacheKey(pipeline, topologyAware, allowShortCircuit); - if (key.endsWith(DomainSocketFactory.FEATURE_FLAG)) { - final Pipeline newPipeline = Pipeline.newBuilder(pipeline).setReplicationConfig( - ReplicationConfig.fromTypeAndFactor(ReplicationType.SHORT_CIRCUIT, - ReplicationFactor.valueOf(pipeline.getReplicationConfig().getReplication()))).build(); - return clientCache.get(key, () -> newClient(newPipeline, localDNCache.get(key))); - } else { - return clientCache.get(key, () -> newClient(pipeline)); - } + return clientCache.get(key, () -> newClient(pipeline, localDNCache.get(key))); } catch (Exception e) { throw new IOException( "Exception getting XceiverClient: " + e, e); diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java index 93f7f30e23f..d82cd08c08e 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfig.java @@ -49,8 +49,6 @@ static ReplicationConfig fromProtoTypeAndFactor( return RatisReplicationConfig.getInstance(factor); case STAND_ALONE: return StandaloneReplicationConfig.getInstance(factor); - case SHORT_CIRCUIT: - return ShortCircuitReplicationConfig.getInstance(factor); default: throw new UnsupportedOperationException( "Not supported replication: " + type); @@ -104,7 +102,6 @@ static ReplicationConfig fromProto( return new ECReplicationConfig(ecConfig); case RATIS: case STAND_ALONE: - case SHORT_CIRCUIT: return fromProtoTypeAndFactor(type, factor); default: throw new UnsupportedOperationException( diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationType.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationType.java index f4dc891f47a..64969eac422 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationType.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationType.java @@ -27,8 +27,7 @@ public enum ReplicationType { RATIS, STAND_ALONE, CHAINED, - EC, - SHORT_CIRCUIT; + EC; public static ReplicationType fromProto( HddsProtos.ReplicationType replicationType) { @@ -44,8 +43,6 @@ public static ReplicationType fromProto( return ReplicationType.CHAINED; case EC: return ReplicationType.EC; - case SHORT_CIRCUIT: - return ReplicationType.SHORT_CIRCUIT; default: throw new IllegalArgumentException( "Unsupported ProtoBuf replication type: " + replicationType); @@ -66,8 +63,6 @@ public static HddsProtos.ReplicationType toProto( return HddsProtos.ReplicationType.CHAINED; case EC: return HddsProtos.ReplicationType.EC; - case SHORT_CIRCUIT: - return HddsProtos.ReplicationType.SHORT_CIRCUIT; default: throw new IllegalArgumentException( "Unsupported replication type: " + replicationType); diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ShortCircuitReplicationConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ShortCircuitReplicationConfig.java deleted file mode 100644 index aa72e8cacf3..00000000000 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ShortCircuitReplicationConfig.java +++ /dev/null @@ -1,129 +0,0 @@ -/** - * 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.hdds.client; - -import com.fasterxml.jackson.annotation.JsonIgnore; -import com.fasterxml.jackson.annotation.JsonProperty; -import net.jcip.annotations.Immutable; -import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor; -import org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationType; - -import java.util.Objects; - -import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE; -import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE; - -/** - * Replication configuration for SHORT_CIRCUIT replication. - */ -@Immutable -public final class ShortCircuitReplicationConfig implements - ReplicatedReplicationConfig { - - private final ReplicationFactor replicationFactor; - private static final String REPLICATION_TYPE = "SHORT_CIRCUIT"; - - private static final ShortCircuitReplicationConfig SHORT_CIRCUIT_ONE_CONFIG = - new ShortCircuitReplicationConfig(ONE); - - private static final ShortCircuitReplicationConfig SHORT_CIRCUIT_THREE_CONFIG = - new ShortCircuitReplicationConfig(THREE); - - /** - * Get an instance of Short-circuit Replication Config with the requested factor. - * The same static instance will be returned for all requests for the same - * factor. - * @param factor Replication Factor requested - * @return ShortCircuitReplicationConfig object of the requested factor - */ - public static ShortCircuitReplicationConfig getInstance( - ReplicationFactor factor) { - if (factor == ONE) { - return SHORT_CIRCUIT_ONE_CONFIG; - } else if (factor == THREE) { - return SHORT_CIRCUIT_THREE_CONFIG; - } - return new ShortCircuitReplicationConfig(factor); - } - - /** - * Use the static getInstance method instead of the private constructor. - * @param replicationFactor - */ - private ShortCircuitReplicationConfig(ReplicationFactor replicationFactor) { - this.replicationFactor = replicationFactor; - } - - @Override - public ReplicationFactor getReplicationFactor() { - return replicationFactor; - } - - @Override - public int getRequiredNodes() { - return replicationFactor.getNumber(); - } - - @Override - @JsonIgnore - public String getReplication() { - return String.valueOf(this.replicationFactor); - } - - @Override - public ReplicationType getReplicationType() { - return ReplicationType.SHORT_CIRCUIT; - } - - /** - * This method is here only to allow the string value for replicationType to - * be output in JSON. - */ - @JsonProperty("replicationType") - public String replicationType() { - return REPLICATION_TYPE; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - ShortCircuitReplicationConfig that = (ShortCircuitReplicationConfig) o; - return replicationFactor == that.replicationFactor; - } - - @Override - public int hashCode() { - return Objects.hash(replicationFactor); - } - - @Override - public String toString() { - return REPLICATION_TYPE + "/" + replicationFactor; - } - - @Override - public String configFormat() { - return toString(); - } -} diff --git a/hadoop-hdds/interface-client/src/main/proto/hdds.proto b/hadoop-hdds/interface-client/src/main/proto/hdds.proto index 63a0a7d7982..1fc5884e24f 100644 --- a/hadoop-hdds/interface-client/src/main/proto/hdds.proto +++ b/hadoop-hdds/interface-client/src/main/proto/hdds.proto @@ -287,7 +287,6 @@ enum ReplicationType { STAND_ALONE = 2; CHAINED = 3; EC = 4; - SHORT_CIRCUIT = 5; NONE = -1; // Invalid Type } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestXceiverClientManagerSC.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestXceiverClientManagerSC.java index 4e11cd1d16c..696ca8aadd2 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestXceiverClientManagerSC.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestXceiverClientManagerSC.java @@ -94,8 +94,7 @@ public void testAllocateShortCircuitClient() throws IOException { assertEquals(client1, client2); clientManager.releaseClient(client1, true); clientManager.releaseClient(client2, true); - // client is still kept in the cache, for create a domain socket connection is expensive. - assertEquals(1, clientManager.getClientCache().size()); + assertEquals(0, clientManager.getClientCache().size()); XceiverClientSpi client3 = clientManager.acquireClientForReadData(container1.getPipeline(), false); assertTrue(client3 instanceof XceiverClientGrpc);