diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java index cb4b8471a00..56cf92653af 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java @@ -32,6 +32,7 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos.DeletedBlocksTransactionSummary; import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.ContainerBalancerStatusInfoResponseProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.DecommissionScmResponseProto; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.SCMListContainerRequestProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.StartContainerBalancerResponseProto; import org.apache.hadoop.hdds.scm.DatanodeAdminError; import org.apache.hadoop.hdds.scm.container.ContainerID; @@ -41,6 +42,8 @@ import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec.ListContainerQuery; import org.apache.hadoop.ozone.upgrade.UpgradeFinalization.StatusAndMessages; /** @@ -115,6 +118,21 @@ void deleteContainer(long containerId, Pipeline pipeline, boolean force) */ void deleteContainer(long containerId, boolean force) throws IOException; + /** + * Lists containers using decoded list parameters. + * + * @param query start container id, count and optional filters + * @return containers matching the query + */ + ContainerListResult listContainer(ListContainerQuery query) throws IOException; + + /** + * Same as {@link #listContainer(ListContainerQuery)} after decoding {@code request}. + */ + default ContainerListResult listContainer(SCMListContainerRequestProto request) throws IOException { + return listContainer(ScmListContainerRequestCodec.fromProto(request)); + } + /** * Lists a range of containers and get their info. * @@ -125,6 +143,7 @@ void deleteContainer(long containerId, Pipeline pipeline, boolean force) * in "ozone.scm.container.list.max.count" and total number of containers. * @throws IOException */ + @Deprecated ContainerListResult listContainer(long startContainerID, int count) throws IOException; @@ -139,6 +158,7 @@ ContainerListResult listContainer(long startContainerID, * in "ozone.scm.container.list.max.count" and total number of containers. * @throws IOException */ + @Deprecated ContainerListResult listContainer(long startContainerID, int count, HddsProtos.LifeCycleState state, HddsProtos.ReplicationType replicationType, @@ -157,6 +177,7 @@ ContainerListResult listContainer(long startContainerID, int count, * in "ozone.scm.container.list.max.count" and total number of containers. * @throws IOException */ + @Deprecated ContainerListResult listContainer(long startContainerID, int count, HddsProtos.LifeCycleState state, HddsProtos.ReplicationType replicationType, diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmListContainerRequestCodec.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmListContainerRequestCodec.java new file mode 100644 index 00000000000..28ad31207d6 --- /dev/null +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/ScmListContainerRequestCodec.java @@ -0,0 +1,249 @@ +/* + * 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.scm.protocol; + +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationType.EC; + +import jakarta.annotation.Nullable; +import org.apache.hadoop.hdds.client.ECReplicationConfig; +import org.apache.hadoop.hdds.client.ReplicatedReplicationConfig; +import org.apache.hadoop.hdds.client.ReplicationConfig; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.SCMListContainerRequestProto; + +/** + * Codec between {@link SCMListContainerRequestProto} and {@link ListContainerQuery} for list containers. + */ +public final class ScmListContainerRequestCodec { + + private ScmListContainerRequestCodec() { + } + + /** + * Immutable parameters for listing containers (encode and decode paths). + * Construct with {@link ListContainerQuery#newBuilder(long, int)} and optional fluent + * setters on {@link ListContainerQuery.Builder}; further filters extend the builder + * without widening the {@link #toProto(ListContainerQuery)} API. + */ + public static final class ListContainerQuery { + private final long startContainerID; + private final int count; + @Nullable + private final String traceId; + @Nullable + private final HddsProtos.LifeCycleState state; + @Nullable + private final HddsProtos.ReplicationFactor factor; + @Nullable + private final HddsProtos.ReplicationType replicationType; + @Nullable + private final ReplicationConfig replicationConfig; + @Nullable + private final Boolean suppressed; + + private ListContainerQuery(Builder builder) { + this.startContainerID = builder.startContainerID; + this.count = builder.count; + this.traceId = builder.traceId; + this.state = builder.state; + this.factor = builder.factor; + this.replicationType = builder.replicationType; + this.replicationConfig = builder.replicationConfig; + this.suppressed = builder.suppressed; + } + + public static Builder newBuilder(long startContainerID, int count) { + return new Builder(startContainerID, count); + } + + public long getStartContainerID() { + return startContainerID; + } + + public int getCount() { + return count; + } + + @Nullable + public String getTraceId() { + return traceId; + } + + @Nullable + public HddsProtos.LifeCycleState getState() { + return state; + } + + @Nullable + public HddsProtos.ReplicationFactor getFactor() { + return factor; + } + + @Nullable + public HddsProtos.ReplicationType getReplicationType() { + return replicationType; + } + + @Nullable + public ReplicationConfig getReplicationConfig() { + return replicationConfig; + } + + @Nullable + public Boolean getSuppressed() { + return suppressed; + } + + /** Fluent builder for {@link ListContainerQuery}. */ + public static final class Builder { + private final long startContainerID; + private final int count; + @Nullable + private String traceId; + @Nullable + private HddsProtos.LifeCycleState state; + @Nullable + private HddsProtos.ReplicationFactor factor; + @Nullable + private HddsProtos.ReplicationType replicationType; + @Nullable + private ReplicationConfig replicationConfig; + @Nullable + private Boolean suppressed; + + private Builder(long startContainerID, int count) { + this.startContainerID = startContainerID; + this.count = count; + } + + public Builder setTraceId(@Nullable String traceIdVal) { + this.traceId = traceIdVal; + return this; + } + + public Builder setState(@Nullable HddsProtos.LifeCycleState stateVal) { + this.state = stateVal; + return this; + } + + public Builder setFactor(@Nullable HddsProtos.ReplicationFactor factorVal) { + this.factor = factorVal; + return this; + } + + public Builder setReplicationType( + @Nullable HddsProtos.ReplicationType replicationTypeVal) { + this.replicationType = replicationTypeVal; + return this; + } + + public Builder setReplicationConfig(@Nullable ReplicationConfig replicationConfigVal) { + this.replicationConfig = replicationConfigVal; + return this; + } + + public Builder setSuppressed(@Nullable Boolean suppressedVal) { + this.suppressed = suppressedVal; + return this; + } + + public ListContainerQuery build() { + return new ListContainerQuery(this); + } + } + } + + public static SCMListContainerRequestProto toProto(ListContainerQuery query) { + SCMListContainerRequestProto.Builder proto = SCMListContainerRequestProto.newBuilder() + .setCount(query.getCount()) + .setStartContainerID(query.getStartContainerID()); + String traceId = query.getTraceId(); + if (traceId != null) { + proto.setTraceID(traceId); + } + HddsProtos.LifeCycleState state = query.getState(); + if (state != null) { + proto.setState(state); + } + ReplicationConfig repConfig = query.getReplicationConfig(); + HddsProtos.ReplicationType replicationType = query.getReplicationType(); + HddsProtos.ReplicationFactor factor = query.getFactor(); + if (repConfig != null) { + HddsProtos.ReplicationType repType = repConfig.getReplicationType(); + if (repType == EC) { + proto.setType(EC); + proto.setEcReplicationConfig(((ECReplicationConfig) repConfig).toProto()); + } else { + proto.setType(repType); + proto.setFactor(((ReplicatedReplicationConfig) repConfig).getReplicationFactor()); + } + } else if (replicationType != null) { + proto.setType(replicationType); + } else if (factor != null) { + proto.setFactor(factor); + } + Boolean suppressed = query.getSuppressed(); + if (suppressed != null) { + proto.setSuppressed(suppressed); + } + return proto.build(); + } + + /** + * Decodes {@link SCMListContainerRequestProto} into {@link ListContainerQuery}. + */ + public static ListContainerQuery fromProto(SCMListContainerRequestProto request) { + long startContainerID = 0L; + if (request.hasStartContainerID()) { + startContainerID = request.getStartContainerID(); + } + int count = request.getCount(); + + ListContainerQuery.Builder builder = ListContainerQuery.newBuilder(startContainerID, count); + + if (request.hasTraceID()) { + builder.setTraceId(request.getTraceID()); + } + if (request.hasState()) { + builder.setState(request.getState()); + } + HddsProtos.ReplicationType replicationType = null; + if (request.hasType()) { + replicationType = request.getType(); + builder.setReplicationType(replicationType); + } + if (replicationType != null) { + if (replicationType == EC) { + if (request.hasEcReplicationConfig()) { + builder.setReplicationConfig(new ECReplicationConfig(request.getEcReplicationConfig())); + } + } else { + if (request.hasFactor()) { + builder.setReplicationConfig(ReplicationConfig.fromProtoTypeAndFactor(replicationType, request.getFactor())); + } + } + } else if (request.hasFactor()) { + builder.setFactor(request.getFactor()); + } + + if (request.hasSuppressed()) { + builder.setSuppressed(request.getSuppressed()); + } + return builder.build(); + } +} diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java index 98f8efa9ae3..f33a78b4f66 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java @@ -35,6 +35,7 @@ import org.apache.hadoop.hdds.protocol.proto.HddsProtos.DeletedBlocksTransactionSummary; import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.ContainerBalancerStatusInfoResponseProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.DecommissionScmResponseProto; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.SCMListContainerRequestProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.StartContainerBalancerResponseProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.Type; import org.apache.hadoop.hdds.scm.DatanodeAdminError; @@ -46,6 +47,7 @@ import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec.ListContainerQuery; import org.apache.hadoop.ozone.upgrade.UpgradeFinalization.StatusAndMessages; import org.apache.hadoop.security.KerberosInfo; import org.apache.hadoop.security.token.Token; @@ -147,110 +149,19 @@ List getExistContainerWithPipelinesInBatch( List containerIDs); /** - * Ask SCM a list of containers with a range of container names - * and the limit of count. - * Search container names between start name(exclusive), and - * use prefix name to filter the result. the max size of the - * searching range cannot exceed the value of count. + * Lists containers using decoded list parameters. * - * @param startContainerID start container ID. - * @param count count, if count {@literal <} 0, the max size is unlimited.( - * Usually the count will be replace with a very big - * value instead of being unlimited in case the db is very big) - * - * @return a list of containers capped by max count allowed - * in "ozone.scm.container.list.max.count" and total number of containers. - * @throws IOException - */ - ContainerListResult listContainer(long startContainerID, - int count) throws IOException; - - /** - * Ask SCM a list of containers with a range of container names - * and the limit of count. - * Search container names between start name(exclusive), and - * use prefix name to filter the result. the max size of the - * searching range cannot exceed the value of count. - * - * @param startContainerID start container ID. - * @param count count, if count {@literal <} 0, the max size is unlimited.( - * Usually the count will be replace with a very big - * value instead of being unlimited in case the db is very big) - * @param state Container with this state will be returned. - * - * @return a list of containers capped by max count allowed - * in "ozone.scm.container.list.max.count" and total number of containers. - * @throws IOException - */ - ContainerListResult listContainer(long startContainerID, - int count, HddsProtos.LifeCycleState state) throws IOException; - - /** - * Ask SCM a list of containers with a range of container names - * and the limit of count. - * Search container names between start name(exclusive), and - * use prefix name to filter the result. the max size of the - * searching range cannot exceed the value of count. - * - * @param startContainerID start container ID. - * @param count count, if count {@literal <} 0, the max size is unlimited.( - * Usually the count will be replace with a very big - * value instead of being unlimited in case the db is very big) - * @param state Container with this state will be returned. - * @param factor Container factor - * @return a list of containers capped by max count allowed - * in "ozone.scm.container.list.max.count" and total number of containers. - * @throws IOException - */ - ContainerListResult listContainer(long startContainerID, - int count, HddsProtos.LifeCycleState state, - HddsProtos.ReplicationFactor factor) throws IOException; - - /** - * Ask SCM for a list of containers with a range of container ID, state - * and replication config, and the limit of count. - * The containers are returned from startID (exclusive), and - * filtered by state and replication config. The returned list is limited to - * count entries. - * - * @param startContainerID start container ID. - * @param count count, if count {@literal <} 0, the max size is unlimited.( - * Usually the count will be replace with a very big - * value instead of being unlimited in case the db is very big) - * @param state Container with this state will be returned. - * @param replicationConfig Replication config for the containers - * @return a list of containers capped by max count allowed - * in "ozone.scm.container.list.max.count" and total number of containers. - * @throws IOException + * @param query start container id, count and optional filters + * @return containers matching the query */ - ContainerListResult listContainer(long startContainerID, - int count, HddsProtos.LifeCycleState state, - HddsProtos.ReplicationType replicationType, - ReplicationConfig replicationConfig) throws IOException; - + ContainerListResult listContainer(ListContainerQuery query) throws IOException; + /** - * Ask SCM for a list of containers with a range of container ID, state - * and replication config, and the limit of count. - * The containers are returned from startID (exclusive), and - * filtered by state and replication config. The returned list is limited to - * count entries. - * - * @param startContainerID start container ID. - * @param count count, if count {@literal <} 0, the max size is unlimited.( - * Usually the count will be replace with a very big - * value instead of being unlimited in case the db is very big) - * @param state Container with this state will be returned. - * @param replicationConfig Replication config for the containers - * @param suppressed container to be suppressed/unsuppressed from report - * @return a list of containers capped by max count allowed - * in "ozone.scm.container.list.max.count" and total number of containers. - * @throws IOException + * Same as {@link #listContainer(ListContainerQuery)} after decoding {@code request}. */ - ContainerListResult listContainer(long startContainerID, - int count, HddsProtos.LifeCycleState state, - HddsProtos.ReplicationType replicationType, - ReplicationConfig replicationConfig, - Boolean suppressed) throws IOException; + default ContainerListResult listContainer(SCMListContainerRequestProto request) throws IOException { + return listContainer(ScmListContainerRequestCodec.fromProto(request)); + } /** * Deletes a container in SCM. diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java index 56e2ef6408f..8154ceefc03 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java @@ -17,7 +17,6 @@ package org.apache.hadoop.hdds.scm.protocolPB; -import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationType.EC; import static org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.SCMCloseContainerResponseProto.Status.CONTAINER_ALREADY_CLOSED; import static org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.SCMCloseContainerResponseProto.Status.CONTAINER_ALREADY_CLOSING; @@ -40,7 +39,6 @@ import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.hdds.client.ECReplicationConfig; -import org.apache.hadoop.hdds.client.ReplicatedReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; @@ -138,6 +136,8 @@ import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec.ListContainerQuery; import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocol; import org.apache.hadoop.hdds.scm.proxy.SCMContainerLocationFailoverProxyProvider; import org.apache.hadoop.hdds.tracing.TracingUtil; @@ -426,71 +426,23 @@ public List getExistContainerWithPipelinesInBatch( return cps; } - /** - * {@inheritDoc} - */ - @Override - public ContainerListResult listContainer(long startContainerID, int count) - throws IOException { - return listContainer(startContainerID, count, null, null, null); - } - - @Override - public ContainerListResult listContainer(long startContainerID, int count, - HddsProtos.LifeCycleState state) throws IOException { - return listContainer(startContainerID, count, state, null, null); - } - - @Override - public ContainerListResult listContainer(long startContainerID, int count, - HddsProtos.LifeCycleState state, - HddsProtos.ReplicationType replicationType, - ReplicationConfig replicationConfig) - throws IOException { - return listContainer(startContainerID, count, state, replicationType, replicationConfig, null); - } - @Override - public ContainerListResult listContainer(long startContainerID, int count, - HddsProtos.LifeCycleState state, - HddsProtos.ReplicationType replicationType, - ReplicationConfig replicationConfig, - Boolean suppressed) - throws IOException { + public ContainerListResult listContainer(ListContainerQuery query) throws IOException { + long startContainerID = query.getStartContainerID(); Preconditions.checkState(startContainerID >= 0, "Container ID cannot be negative."); - Preconditions.checkState(count > 0, + Preconditions.checkState(query.getCount() > 0, "Container count must be greater than 0."); - SCMListContainerRequestProto.Builder builder = SCMListContainerRequestProto - .newBuilder(); - builder.setStartContainerID(startContainerID); - builder.setCount(count); - builder.setTraceID(TracingUtil.exportCurrentSpan()); - if (suppressed != null) { - builder.setSuppressed(suppressed); - } - if (state != null) { - builder.setState(state); - } - if (replicationConfig != null) { - if (replicationConfig.getReplicationType() == EC) { - builder.setType(EC); - builder.setEcReplicationConfig( - ((ECReplicationConfig)replicationConfig).toProto()); - } else { - builder.setType(replicationConfig.getReplicationType()); - builder.setFactor(((ReplicatedReplicationConfig)replicationConfig) - .getReplicationFactor()); - } - } else if (replicationType != null) { - builder.setType(replicationType); - } - - SCMListContainerRequestProto request = builder.build(); + SCMListContainerRequestProto withTrace = ScmListContainerRequestCodec.toProto(query).toBuilder() + .setTraceID(TracingUtil.exportCurrentSpan()) + .build(); + return submitListContainer(withTrace); + } + private ContainerListResult submitListContainer(SCMListContainerRequestProto request) throws IOException { SCMListContainerResponseProto response = submitRequest(Type.ListContainer, - builder1 -> builder1.setScmListContainerRequest(request)) + builder -> builder.setScmListContainerRequest(request)) .getScmListContainerResponse(); List containerList = new ArrayList<>(); for (HddsProtos.ContainerInfoProto containerInfoProto : response @@ -505,15 +457,6 @@ public ContainerListResult listContainer(long startContainerID, int count, } } - @Deprecated - @Override - public ContainerListResult listContainer(long startContainerID, int count, - HddsProtos.LifeCycleState state, HddsProtos.ReplicationFactor factor) - throws IOException { - throw new UnsupportedOperationException("Should no longer be called from " + - "the client side"); - } - /** * Ask SCM to delete a container by name. SCM will remove * the container mapping in its database. diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java index 73bf92e9cd5..b71ba5225ca 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocolServerSideTranslatorPB.java @@ -40,7 +40,6 @@ import java.util.Optional; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.annotation.InterfaceAudience; -import org.apache.hadoop.hdds.client.ECReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; @@ -862,53 +861,7 @@ public GetContainerWithPipelineResponseProto getContainerWithPipeline( public SCMListContainerResponseProto listContainer( SCMListContainerRequestProto request) throws IOException { - long startContainerID = 0; - int count = -1; - - // Arguments check. - if (request.hasStartContainerID()) { - // End container name is given. - startContainerID = request.getStartContainerID(); - } - count = request.getCount(); - HddsProtos.LifeCycleState state = null; - HddsProtos.ReplicationFactor factor = null; - HddsProtos.ReplicationType replicationType = null; - ReplicationConfig repConfig = null; - if (request.hasState()) { - state = request.getState(); - } - if (request.hasType()) { - replicationType = request.getType(); - } - if (replicationType != null) { - // This must come from an upgraded client as the older version never - // passed Type. Therefore, we must check for replicationConfig. - if (replicationType == HddsProtos.ReplicationType.EC) { - if (request.hasEcReplicationConfig()) { - repConfig = new ECReplicationConfig(request.getEcReplicationConfig()); - } - } else { - if (request.hasFactor()) { - repConfig = ReplicationConfig - .fromProtoTypeAndFactor(request.getType(), request.getFactor()); - } - } - } else if (request.hasFactor()) { - factor = request.getFactor(); - } - // Filter by suppressed: true (suppressed only), false (unsuppressed only) or null (display all). - Boolean suppressed = request.hasSuppressed() ? request.getSuppressed() : null; - - ContainerListResult containerListAndTotalCount; - if (factor != null) { - // Call from a legacy client - containerListAndTotalCount = - impl.listContainer(startContainerID, count, state, factor); - } else { - containerListAndTotalCount = - impl.listContainer(startContainerID, count, state, replicationType, repConfig, suppressed); - } + ContainerListResult containerListAndTotalCount = impl.listContainer(request); SCMListContainerResponseProto.Builder builder = SCMListContainerResponseProto.newBuilder(); for (ContainerInfo container : containerListAndTotalCount.getContainerInfoList()) { diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java index 156eed688d8..00528ffef52 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java @@ -99,6 +99,7 @@ import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.scm.pipeline.PipelineManager; import org.apache.hadoop.hdds.scm.pipeline.PipelineNotFoundException; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec.ListContainerQuery; import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocol; import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocolServerSideTranslatorPB; import org.apache.hadoop.hdds.scm.protocolPB.StorageContainerLocationProtocolPB; @@ -440,63 +441,25 @@ private boolean hasRequiredReplicas(ContainerInfo contInfo) { } /** - * Lists a range of containers and get their info. - * - * @param startContainerID start containerID. - * @param count count must be {@literal >} 0. - * - * @return a list of containers capped by max count allowed - * in "ozone.scm.container.list.max.count" and total number of containers. - * @throws IOException + * {@inheritDoc} */ @Override - public ContainerListResult listContainer(long startContainerID, - int count) throws IOException { - return listContainer(startContainerID, count, null, null, null, null); + public ContainerListResult listContainer(ListContainerQuery query) { + return listContainerWithQuery(query); } /** - * Lists a range of containers and get their info. - * - * @param startContainerID start containerID. - * @param count count must be {@literal >} 0. - * @param state Container with this state will be returned. - * - * @return a list of containers capped by max count allowed - * in "ozone.scm.container.list.max.count" and total number of containers. - * @throws IOException + * Applies filters from {@link ListContainerQuery}. */ - @Override - public ContainerListResult listContainer(long startContainerID, - int count, HddsProtos.LifeCycleState state) throws IOException { - return listContainer(startContainerID, count, state, null, null, null); - } + private ContainerListResult listContainerWithQuery(ListContainerQuery query) { + long startContainerID = query.getStartContainerID(); + int count = query.getCount(); + HddsProtos.LifeCycleState state = query.getState(); + HddsProtos.ReplicationFactor factor = query.getFactor(); + HddsProtos.ReplicationType replicationType = query.getReplicationType(); + ReplicationConfig repConfig = query.getReplicationConfig(); + Boolean suppressed = query.getSuppressed(); - /** - * Lists a range of containers and get their info. - * - * @param startContainerID start containerID. - * @param count count must be {@literal >} 0. - * @param state Container with this state will be returned. - * @param factor Container factor. - * @return a list of containers capped by max count allowed - * in "ozone.scm.container.list.max.count" and total number of containers. - * @throws IOException - */ - @Override - @Deprecated - public ContainerListResult listContainer(long startContainerID, - int count, HddsProtos.LifeCycleState state, - HddsProtos.ReplicationFactor factor) throws IOException { - return listContainerInternal(startContainerID, count, state, factor, null, null, null); - } - - private ContainerListResult listContainerInternal(long startContainerID, int count, - HddsProtos.LifeCycleState state, - HddsProtos.ReplicationFactor factor, - HddsProtos.ReplicationType replicationType, - ReplicationConfig repConfig, - Boolean suppressed) throws IOException { boolean auditSuccess = true; Map auditMap = buildAuditMap(startContainerID, count, state, factor, replicationType, repConfig, suppressed); @@ -585,46 +548,6 @@ private Map buildAuditMap(long startContainerID, int count, return auditMap; } - /** - * Lists a range of containers and get their info. - * - * @param startContainerID start containerID. - * @param count count must be {@literal >} 0. - * @param state Container with this state will be returned. - * @param repConfig Replication Config for the container. - * @return a list of containers capped by max count allowed - * in "ozone.scm.container.list.max.count" and total number of containers. - * @throws IOException - */ - @Override - public ContainerListResult listContainer(long startContainerID, - int count, HddsProtos.LifeCycleState state, - HddsProtos.ReplicationType replicationType, - ReplicationConfig repConfig) throws IOException { - return listContainerInternal(startContainerID, count, state, null, replicationType, repConfig, null); - } - - /** - * Lists a range of containers and get their info. - * - * @param startContainerID start containerID. - * @param count count must be {@literal >} 0. - * @param state Container with this state will be returned. - * @param repConfig Replication Config for the container. - * @param suppressed container to be suppressed/unsuppressed from report - * @return a list of containers capped by max count allowed - * in "ozone.scm.container.list.max.count" and total number of containers. - * @throws IOException - */ - @Override - public ContainerListResult listContainer(long startContainerID, - int count, HddsProtos.LifeCycleState state, - HddsProtos.ReplicationType replicationType, - ReplicationConfig repConfig, - Boolean suppressed) throws IOException { - return listContainerInternal(startContainerID, count, state, null, replicationType, repConfig, suppressed); - } - @Override public void deleteContainer(long containerID) throws IOException { Map auditMap = Maps.newHashMap(); diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMClientProtocolServer.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMClientProtocolServer.java index 7d2f399d1fa..b30b3a23f19 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMClientProtocolServer.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestSCMClientProtocolServer.java @@ -45,6 +45,7 @@ import org.apache.hadoop.hdds.scm.ha.SCMHAManagerStub; import org.apache.hadoop.hdds.scm.ha.SCMNodeDetails; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec.ListContainerQuery; import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocolServerSideTranslatorPB; import org.apache.hadoop.hdds.utils.ProtocolMessageMetrics; import org.apache.hadoop.ozone.container.common.SCMTestUtils; @@ -134,11 +135,12 @@ public void testScmListContainer() throws Exception { new SCMClientProtocolServer(new OzoneConfiguration(), mockStorageContainerManager(), mock(ReconfigurationHandler.class)); try { - assertEquals(10, scmServer.listContainer(1, 10, - null, HddsProtos.ReplicationType.RATIS, null).getContainerInfoList().size()); - // Test call from a legacy client, which uses a different method of listContainer - assertEquals(10, scmServer.listContainer(1, 10, null, - HddsProtos.ReplicationFactor.THREE).getContainerInfoList().size()); + assertEquals(10, scmServer.listContainer(ListContainerQuery.newBuilder(1, 10) + .setReplicationType(HddsProtos.ReplicationType.RATIS).build()) + .getContainerInfoList().size()); + assertEquals(10, scmServer.listContainer(ListContainerQuery.newBuilder(1, 10) + .setFactor(HddsProtos.ReplicationFactor.THREE).build()) + .getContainerInfoList().size()); } finally { scmServer.stop(); } diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java index c1973891d0a..79fda7392d9 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java +++ b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java @@ -56,6 +56,7 @@ import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport; import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerWithPipeline; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec.ListContainerQuery; import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocol; import org.apache.hadoop.hdds.scm.protocolPB.StorageContainerLocationProtocolClientSideTranslatorPB.ScmNodeTarget; import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls; @@ -360,49 +361,58 @@ public void deleteContainer(long containerID, boolean force) ContainerWithPipeline info = getContainerWithPipeline(containerID); deleteContainer(containerID, info.getPipeline(), force); } - + @Override - public ContainerListResult listContainer(long startContainerID, - int count) throws IOException { + public ContainerListResult listContainer(ListContainerQuery query) throws IOException { + int count = query.getCount(); if (count > maxCountOfContainerList) { LOG.warn("Attempting to list {} containers. However, this exceeds" + " the cluster's current limit of {}. The results will be capped at the" + " maximum allowed count.", count, maxCountOfContainerList); - count = maxCountOfContainerList; + query = ListContainerQuery.newBuilder(query.getStartContainerID(), maxCountOfContainerList) + .setState(query.getState()) + .setFactor(query.getFactor()) + .setReplicationType(query.getReplicationType()) + .setReplicationConfig(query.getReplicationConfig()) + .setSuppressed(query.getSuppressed()) + .build(); } - return storageContainerLocationClient.listContainer( - startContainerID, count); + return storageContainerLocationClient.listContainer(query); + } + + @Deprecated + @Override + public ContainerListResult listContainer(long startContainerID, + int count) throws IOException { + return listContainer(ListContainerQuery.newBuilder(startContainerID, count).build()); } + @Deprecated @Override public ContainerListResult listContainer(long startContainerID, int count, HddsProtos.LifeCycleState state, HddsProtos.ReplicationType repType, ReplicationConfig replicationConfig) throws IOException { - if (count > maxCountOfContainerList) { - LOG.warn("Attempting to list {} containers. However, this exceeds" + - " the cluster's current limit of {}. The results will be capped at the" + - " maximum allowed count.", count, maxCountOfContainerList); - count = maxCountOfContainerList; - } - return storageContainerLocationClient.listContainer( - startContainerID, count, state, repType, replicationConfig); + return listContainer(ListContainerQuery.newBuilder(startContainerID, count) + .setState(state) + .setReplicationType(repType) + .setReplicationConfig(replicationConfig) + .build()); } + @Deprecated @Override public ContainerListResult listContainer(long startContainerID, int count, HddsProtos.LifeCycleState state, - HddsProtos.ReplicationType repType, - ReplicationConfig replicationConfig, - Boolean suppressed) throws IOException { - if (count > maxCountOfContainerList) { - LOG.warn("Attempting to list {} containers. However, this exceeds" + - " the cluster's current limit of {}. The results will be capped at the" + - " maximum allowed count.", count, maxCountOfContainerList); - count = maxCountOfContainerList; - } - return storageContainerLocationClient.listContainer( - startContainerID, count, state, repType, replicationConfig, suppressed); + HddsProtos.ReplicationType repType, + ReplicationConfig replicationConfig, + Boolean suppressed) throws IOException { + return listContainer(ListContainerQuery.newBuilder(startContainerID, count) + .setState(state) + .setReplicationType(repType) + .setReplicationConfig(replicationConfig) + .setSuppressed(suppressed) + .build()); } @Override diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java index 0cc4b8bdee1..8294646e0db 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java +++ b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java @@ -37,6 +37,7 @@ import org.apache.hadoop.hdds.scm.client.ScmClient; import org.apache.hadoop.hdds.scm.container.ContainerInfo; import org.apache.hadoop.hdds.scm.container.ContainerListResult; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec.ListContainerQuery; import org.apache.hadoop.hdds.server.JsonUtils; import picocli.CommandLine.Command; import picocli.CommandLine.Help.Visibility; @@ -133,8 +134,12 @@ public void execute(ScmClient scmClient) throws IOException { count = maxCountAllowed; } - ContainerListResult containerListResult = - scmClient.listContainer(startId, count, state, type, repConfig, suppressed); + ContainerListResult containerListResult = scmClient.listContainer(ListContainerQuery.newBuilder(startId, count) + .setState(state) + .setReplicationType(type) + .setReplicationConfig(repConfig) + .setSuppressed(suppressed) + .build()); writeContainers(sequenceWriter, containerListResult.getContainerInfoList()); @@ -173,8 +178,12 @@ private void listAllContainers(ScmClient scmClient, SequenceWriter writer, int fetchedCount; do { - ContainerListResult result = - scmClient.listContainer(currentStartId, batchSize, state, type, repConfig, suppressed); + ContainerListResult result = scmClient.listContainer(ListContainerQuery.newBuilder(currentStartId, batchSize) + .setState(state) + .setReplicationType(type) + .setReplicationConfig(repConfig) + .setSuppressed(suppressed) + .build()); fetchedCount = result.getContainerInfoList().size(); writeContainers(writer, result.getContainerInfoList()); diff --git a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestContainerReportSuppressOptions.java b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestContainerReportSuppressOptions.java index 2e4ef55b8ec..0f77a60b11d 100644 --- a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestContainerReportSuppressOptions.java +++ b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestContainerReportSuppressOptions.java @@ -23,9 +23,8 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -47,6 +46,7 @@ import org.apache.hadoop.hdds.scm.container.ContainerListResult; import org.apache.hadoop.hdds.scm.container.ReplicationManagerReport; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec.ListContainerQuery; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.MethodOrderer; @@ -82,9 +82,9 @@ public void setup() throws IOException { when(scmClient.getReplicationManagerReport()).thenAnswer(inv -> createMockReport()); // Mock listContainer - when(scmClient.listContainer(anyLong(), anyInt(), eq(null), eq(null), eq(null), eq(true))) + when(scmClient.listContainer(argThat((ListContainerQuery q) -> isSuppressedFilter(q)))) .thenAnswer(inv -> listSuppressedContainers()); - when(scmClient.listContainer(anyLong(), anyInt(), eq(null), eq(null), eq(null), eq(false))) + when(scmClient.listContainer(argThat((ListContainerQuery q) -> isNonSuppressedFilter(q)))) .thenAnswer(inv -> listNonSuppressedContainers()); // Mock suppress/unsuppress @@ -307,4 +307,12 @@ private ContainerListResult listNonSuppressedContainers() { } return new ContainerListResult(nonSuppressed, nonSuppressed.size()); } + + private static boolean isSuppressedFilter(ListContainerQuery query) { + return query != null && Boolean.TRUE.equals(query.getSuppressed()); + } + + private static boolean isNonSuppressedFilter(ListContainerQuery query) { + return query != null && Boolean.FALSE.equals(query.getSuppressed()); + } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestContainerOperations.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestContainerOperations.java index 8f46d73e17b..3bdaa667996 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestContainerOperations.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestContainerOperations.java @@ -49,6 +49,7 @@ import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.scm.pipeline.PipelineNotFoundException; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec.ListContainerQuery; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls; import org.apache.hadoop.hdds.utils.IOUtils; @@ -146,7 +147,7 @@ public void testListContainerExceedMaxAllowedCountOperations() throws Exception .ONE, OzoneConsts.OZONE); } - int count = storageClient.listContainer(0, CONTAINER_LIST_LIMIT + 1) + int count = storageClient.listContainer(ListContainerQuery.newBuilder(0, CONTAINER_LIST_LIMIT + 1).build()) .getContainerInfoList() .size(); assertEquals(CONTAINER_LIST_LIMIT, count); diff --git a/hadoop-ozone/vapor/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java b/hadoop-ozone/vapor/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java index 6e1be57d420..554204bc731 100644 --- a/hadoop-ozone/vapor/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java +++ b/hadoop-ozone/vapor/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java @@ -40,6 +40,7 @@ import org.apache.hadoop.hdds.scm.cli.ContainerOperationClient; import org.apache.hadoop.hdds.scm.container.ContainerInfo; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; +import org.apache.hadoop.hdds.scm.protocol.ScmListContainerRequestCodec.ListContainerQuery; import org.apache.hadoop.hdds.utils.HddsServerUtil; import org.apache.hadoop.hdfs.server.datanode.StorageLocation; import org.apache.hadoop.ozone.container.checksum.ContainerChecksumTreeManager; @@ -127,7 +128,8 @@ public Void replicate() throws Exception { new ContainerOperationClient(conf); final List containerInfos = - containerOperationClient.listContainer(0L, 1_000_000).getContainerInfoList(); + containerOperationClient.listContainer(ListContainerQuery.newBuilder(0L, 1_000_000).build()) + .getContainerInfoList(); //logic same as the download+import on the destination datanode initializeReplicationSupervisor(conf, containerInfos.size() * 2);