From e600d15809e6811ab98a51343c8b156bc0ea875f Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Mon, 15 Jul 2024 21:17:43 +0530 Subject: [PATCH 01/11] HDDS-10376. Add a Datanode API to supply a merkle tree for a given container. --- .../ContainerCommandResponseBuilders.java | 11 + .../scm/storage/ContainerProtocolCalls.java | 28 +++ .../apache/hadoop/ozone/audit/DNAction.java | 3 +- .../ContainerChecksumTreeManager.java | 24 +++ .../checksum/DNContainerOperationClient.java | 119 +++++++++++ .../container/common/impl/HddsDispatcher.java | 1 + .../container/common/interfaces/Handler.java | 7 +- .../ec/reconstruction/TokenHelper.java | 12 +- .../container/keyvalue/KeyValueHandler.java | 52 ++++- .../container/ozoneimpl/OzoneContainer.java | 14 +- .../checksum/TestContainerMerkleTree.java | 5 +- .../common/TestBlockDeletingService.java | 3 +- .../common/impl/TestHddsDispatcher.java | 6 +- .../common/interfaces/TestHandler.java | 2 +- .../main/proto/DatanodeClientProtocol.proto | 11 + .../metrics/TestContainerMetrics.java | 2 +- .../container/server/TestContainerServer.java | 3 +- .../server/TestSecureContainerServer.java | 2 +- .../TestContainerCommandReconciliation.java | 196 ++++++++++++++++++ .../debug/container/ContainerCommands.java | 4 +- .../freon/ClosedContainerReplicator.java | 4 +- 21 files changed, 481 insertions(+), 28 deletions(-) create mode 100644 hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java create mode 100644 hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java index 25b26aab3ca..280b3d858cd 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java @@ -335,6 +335,17 @@ public static ContainerCommandResponseProto getEchoResponse( .build(); } + public static ContainerCommandResponseProto getReadContainerMerkleTreeResponse( + ContainerCommandRequestProto request, byte[] checksumBytes) { + + ContainerProtos.ReadContainerMerkleTreeResponseProto.Builder containerMerkleTree = + ContainerProtos.ReadContainerMerkleTreeResponseProto.newBuilder() + .setContainerID(request.getContainerID()) + .setContainerMerkleTree(ByteString.copyFrom(checksumBytes)); + return getSuccessResponseBuilder(request) + .setReadContainerMerkleTree(containerMerkleTree).build(); + } + private ContainerCommandResponseBuilders() { throw new UnsupportedOperationException("no instances"); } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java index 66c8459a01a..2866a80be45 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java @@ -723,6 +723,34 @@ public static EchoResponseProto echo(XceiverClientSpi client, String encodedCont return response.getEcho(); } + public static ContainerProtos.ReadContainerMerkleTreeResponseProto readContainerMerkleTree( + XceiverClientSpi client, long containerID, String encodedContainerID) throws IOException { + ContainerProtos.ReadContainerMerkleTreeRequestProto containerMerkleTreeRequestProto = + ContainerProtos.ReadContainerMerkleTreeRequestProto + .newBuilder() + .setContainerID(containerID) + .build(); + String id = client.getPipeline().getClosestNode().getUuidString(); + + ContainerCommandRequestProto.Builder builder = ContainerCommandRequestProto + .newBuilder() + .setCmdType(Type.ReadContainerMerkleTree) + .setContainerID(containerID) + .setDatanodeUuid(id) + .setReadContainerMerkleTree(containerMerkleTreeRequestProto); + if (encodedContainerID != null) { + builder.setEncodedToken(encodedContainerID); + } + String traceId = TracingUtil.exportCurrentSpan(); + if (traceId != null) { + builder.setTraceID(traceId); + } + ContainerCommandRequestProto request = builder.build(); + ContainerCommandResponseProto response = + client.sendCommand(request, getValidatorList()); + return response.getReadContainerMerkleTree(); + } + /** * Validates a response from a container protocol call. Any non-successful * return code is mapped to a corresponding exception and thrown. diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/audit/DNAction.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/audit/DNAction.java index 3d6c9795b47..6702a988938 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/audit/DNAction.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/audit/DNAction.java @@ -40,7 +40,8 @@ public enum DNAction implements AuditAction { CLOSE_CONTAINER, GET_COMMITTED_BLOCK_LENGTH, STREAM_INIT, - ECHO; + ECHO, + READ_CONTAINER_MERKLE_TREE; @Override public String getAction() { diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java index 939c6d08b33..c54e1af468f 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java @@ -16,12 +16,14 @@ */ package org.apache.hadoop.ozone.container.checksum; +import org.apache.commons.io.IOUtils; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; import java.io.File; import java.io.FileInputStream; +import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.util.SortedSet; @@ -157,6 +159,28 @@ private void write(KeyValueContainerData data, ContainerProtos.ContainerChecksum } } + public byte[] readChecksumFileAsBytes(KeyValueContainerData data) { + long containerID = data.getContainerID(); + Lock readLock = getReadLock(containerID); + readLock.lock(); + try { + File checksumFile = getContainerChecksumFile(data); + + try (FileInputStream inStream = new FileInputStream(checksumFile)) { + return IOUtils.toByteArray(inStream); + } catch (FileNotFoundException ex) { + LOG.info("No checksum file currently exists for container {} at the path {}. Returning an empty instance.", + containerID, checksumFile, ex); + } catch (IOException ex) { + LOG.info("Error occured when reading checksum file for container {} at the path {}. " + + "Returning an empty instance.", containerID, checksumFile, ex); + } + return null; + } finally { + readLock.unlock(); + } + } + /** * This class represents the difference between our replica of a container and a peer's replica of a container. * It summarizes the operations we need to do to reconcile our replica with the peer replica it was compared to. diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java new file mode 100644 index 00000000000..c1cb21f6b78 --- /dev/null +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java @@ -0,0 +1,119 @@ +/* + * 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.ozone.container.checksum; + +import com.google.common.collect.ImmutableList; +import org.apache.hadoop.hdds.client.StandaloneReplicationConfig; +import org.apache.hadoop.hdds.conf.ConfigurationSource; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.scm.XceiverClientManager; +import org.apache.hadoop.hdds.scm.XceiverClientSpi; +import org.apache.hadoop.hdds.scm.client.ClientTrustManager; +import org.apache.hadoop.hdds.scm.container.ContainerID; +import org.apache.hadoop.hdds.scm.pipeline.Pipeline; +import org.apache.hadoop.hdds.scm.pipeline.PipelineID; +import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls; +import org.apache.hadoop.hdds.security.SecurityConfig; +import org.apache.hadoop.hdds.security.symmetric.SecretKeySignerClient; +import org.apache.hadoop.hdds.security.x509.certificate.client.CACertificateProvider; +import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; +import org.apache.hadoop.hdds.utils.HAUtils; +import org.apache.hadoop.ozone.OzoneSecurityUtil; +import jakarta.annotation.Nonnull; +import org.apache.hadoop.ozone.container.ec.reconstruction.TokenHelper; +import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; + +import static org.apache.hadoop.ozone.container.ec.reconstruction.TokenHelper.encode; + +/** + * This class wraps necessary container-level rpc calls for container reconcilitaion. + * - ReadContainerMerkleTree + */ +public class DNContainerOperationClient implements AutoCloseable { + + private static final Logger LOG = + LoggerFactory.getLogger(DNContainerOperationClient.class); + private final TokenHelper tokenHelper; + private final XceiverClientManager xceiverClientManager; + + public DNContainerOperationClient(ConfigurationSource conf, + CertificateClient certificateClient, + SecretKeySignerClient secretKeyClient) throws IOException { + this.tokenHelper = new TokenHelper(new SecurityConfig(conf), secretKeyClient); + this.xceiverClientManager = createClientManager(conf, certificateClient); + } + + @Nonnull + private static XceiverClientManager createClientManager( + ConfigurationSource conf, CertificateClient certificateClient) + throws IOException { + ClientTrustManager trustManager = null; + if (OzoneSecurityUtil.isSecurityEnabled(conf)) { + CACertificateProvider localCaCerts = + () -> HAUtils.buildCAX509List(certificateClient, conf); + CACertificateProvider remoteCacerts = + () -> HAUtils.buildCAX509List(null, conf); + trustManager = new ClientTrustManager(remoteCacerts, localCaCerts); + } + return new XceiverClientManager(conf, + new XceiverClientManager.XceiverClientManagerConfigBuilder() + .setMaxCacheSize(100).setStaleThresholdMs(10 * 1000).build(), + trustManager); + } + + public XceiverClientManager getXceiverClientManager() { + return xceiverClientManager; + } + + public ByteString readContainerMerkleTree(long containerId, DatanodeDetails dn) + throws IOException { + XceiverClientSpi xceiverClient = this.xceiverClientManager.acquireClient(createSingleNodePipeline(dn)); + try { + String containerToken = encode(tokenHelper.getContainerToken( + ContainerID.valueOf(containerId))); + ContainerProtos.ReadContainerMerkleTreeResponseProto response = + ContainerProtocolCalls.readContainerMerkleTree(xceiverClient, + containerId, containerToken); + return response.getContainerMerkleTree(); + } finally { + this.xceiverClientManager.releaseClient(xceiverClient, false); + } + } + + private Pipeline createSingleNodePipeline(DatanodeDetails dn) { + return Pipeline.newBuilder() + .setNodes(ImmutableList.of(dn)) + .setId(PipelineID.valueOf(dn.getUuid())) + .setState(Pipeline.PipelineState.CLOSED) + .setReplicationConfig(StandaloneReplicationConfig.getInstance( + HddsProtos.ReplicationFactor.ONE)).build(); + } + + @Override + public void close() throws IOException { + if (xceiverClientManager != null) { + xceiverClientManager.close(); + } + } +} diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java index e494243ccc1..f84673f2fe6 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java @@ -815,6 +815,7 @@ private static DNAction getAuditAction(Type cmdType) { case GetCommittedBlockLength : return DNAction.GET_COMMITTED_BLOCK_LENGTH; case StreamInit : return DNAction.STREAM_INIT; case Echo : return DNAction.ECHO; + case ReadContainerMerkleTree : return DNAction.READ_CONTAINER_MERKLE_TREE; default : LOG.debug("Invalid command type - {}", cmdType); return null; diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java index 179274f2c02..78c9b1ade97 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java @@ -30,6 +30,7 @@ import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandResponseProto; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerType; import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException; +import org.apache.hadoop.ozone.container.checksum.DNContainerOperationClient; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics; import org.apache.hadoop.ozone.container.common.impl.ContainerData; @@ -70,16 +71,18 @@ protected Handler(ConfigurationSource config, String datanodeId, this.icrSender = icrSender; } + @SuppressWarnings("checkstyle:ParameterNumber") public static Handler getHandlerForContainerType( final ContainerType containerType, final ConfigurationSource config, final String datanodeId, final ContainerSet contSet, final VolumeSet volumeSet, final ContainerMetrics metrics, - IncrementalReportSender icrSender) { + IncrementalReportSender icrSender, + DNContainerOperationClient dnClient) { switch (containerType) { case KeyValueContainer: return new KeyValueHandler(config, datanodeId, contSet, volumeSet, metrics, - icrSender); + icrSender, dnClient); default: throw new IllegalArgumentException("Handler for ContainerType: " + containerType + "doesn't exist."); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/TokenHelper.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/TokenHelper.java index d916300a7c2..2f42a1161f1 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/TokenHelper.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/TokenHelper.java @@ -40,7 +40,7 @@ /** * Wraps block and container token managers for datanode. */ -class TokenHelper { +public class TokenHelper { private final OzoneBlockTokenSecretManager blockTokenMgr; private final ContainerTokenSecretManager containerTokenMgr; @@ -48,8 +48,8 @@ class TokenHelper { private static final Set MODES = EnumSet.of(READ, WRITE, DELETE); - TokenHelper(SecurityConfig securityConfig, - SecretKeySignerClient secretKeyClient) throws IOException { + public TokenHelper(SecurityConfig securityConfig, + SecretKeySignerClient secretKeyClient) throws IOException { boolean blockTokenEnabled = securityConfig.isBlockTokenEnabled(); boolean containerTokenEnabled = securityConfig.isContainerTokenEnabled(); @@ -83,19 +83,19 @@ class TokenHelper { } } - Token getBlockToken(BlockID blockID, long length) { + public Token getBlockToken(BlockID blockID, long length) { return blockTokenMgr != null ? blockTokenMgr.generateToken(user, blockID, MODES, length) : null; } - Token getContainerToken(ContainerID containerID) { + public Token getContainerToken(ContainerID containerID) { return containerTokenMgr != null ? containerTokenMgr.generateToken(user, containerID) : null; } - static String encode(Token token) throws IOException { + public static String encode(Token token) throws IOException { return token != null ? token.encodeToUrlString() : null; } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java index 2843fe3bcff..d126aa9d489 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java @@ -62,6 +62,8 @@ import org.apache.hadoop.ozone.common.ChunkBuffer; import org.apache.hadoop.ozone.common.OzoneChecksumException; import org.apache.hadoop.ozone.common.utils.BufferUtils; +import org.apache.hadoop.ozone.container.checksum.ContainerChecksumTreeManager; +import org.apache.hadoop.ozone.container.checksum.DNContainerOperationClient; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.helpers.ChunkInfo; import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics; @@ -110,6 +112,7 @@ import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getListBlockResponse; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getPutFileResponseSuccess; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getReadChunkResponse; +import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getReadContainerMerkleTreeResponse; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getReadContainerResponse; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getSuccessResponse; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getSuccessResponseBuilder; @@ -142,6 +145,8 @@ public class KeyValueHandler extends Handler { private final boolean validateChunkChecksumData; // A striped lock that is held during container creation. private final Striped containerCreationLocks; + private final ContainerChecksumTreeManager checksumManager; + private DNContainerOperationClient dnClient; public KeyValueHandler(ConfigurationSource config, String datanodeId, @@ -155,6 +160,8 @@ public KeyValueHandler(ConfigurationSource config, DatanodeConfiguration.class).isChunkDataValidationCheck(); chunkManager = ChunkManagerFactory.createChunkManager(config, blockManager, volSet); + checksumManager = new ContainerChecksumTreeManager( + config.getObject(DatanodeConfiguration.class)); try { volumeChoosingPolicy = VolumeChoosingPolicyFactory.getPolicy(conf); } catch (Exception e) { @@ -186,6 +193,17 @@ public KeyValueHandler(ConfigurationSource config, .createByteBufferConversion(isUnsafeByteBufferConversionEnabled); } + public KeyValueHandler(ConfigurationSource config, + String datanodeId, + ContainerSet contSet, + VolumeSet volSet, + ContainerMetrics metrics, + IncrementalReportSender icrSender, + DNContainerOperationClient dnClient) { + this(config, datanodeId, contSet, volSet, metrics, icrSender); + this.dnClient = dnClient; + } + @VisibleForTesting public VolumeChoosingPolicy getVolumeChoosingPolicyForTesting() { return volumeChoosingPolicy; @@ -280,6 +298,8 @@ static ContainerCommandResponseProto dispatchRequest(KeyValueHandler handler, return handler.handleGetCommittedBlockLength(request, kvContainer); case Echo: return handler.handleEcho(request, kvContainer); + case ReadContainerMerkleTree: + return handler.handleReadContainerMerkleTree(request, kvContainer); default: return null; } @@ -295,6 +315,11 @@ public BlockManager getBlockManager() { return this.blockManager; } + @VisibleForTesting + public ContainerChecksumTreeManager getChecksumManager() { + return this.checksumManager; + } + ContainerCommandResponseProto handleStreamInit( ContainerCommandRequestProto request, KeyValueContainer kvContainer, DispatcherContext dispatcherContext) { @@ -573,6 +598,22 @@ ContainerCommandResponseProto handleEcho( return getEchoResponse(request); } + ContainerCommandResponseProto handleReadContainerMerkleTree( + ContainerCommandRequestProto request, KeyValueContainer kvContainer) { + + if (!request.hasReadContainerMerkleTree()) { + if (LOG.isDebugEnabled()) { + LOG.debug("Malformed Read Container Merkle tree request. trace ID: {}", + request.getTraceID()); + } + return malformedRequest(request); + } + + KeyValueContainerData containerData = kvContainer.getContainerData(); + byte[] checksumBytes = checksumManager.readChecksumFileAsBytes(containerData); + return getReadContainerMerkleTreeResponse(request, checksumBytes); + } + /** * Handle Get Block operation. Calls BlockManager to process the request. */ @@ -1165,7 +1206,16 @@ public void deleteContainer(Container container, boolean force) @Override public void reconcileContainer(Container container, List peers) throws IOException { - // TODO Just a deterministic placeholder hash for testing until actual implementation is finished. + for (DatanodeDetails dn : peers) { + KeyValueContainerData containerData = + (KeyValueContainerData) container.getContainerData(); + ByteString containerMerkleTree = + dnClient.readContainerMerkleTree(containerData.getContainerID(), dn); + ContainerProtos.ContainerChecksumInfo containerChecksumInfo = + ContainerProtos.ContainerChecksumInfo.parseFrom(containerMerkleTree); + LOG.debug("Container Merkle Tree for container {} is {}", + containerData.getContainerID(), containerChecksumInfo.getContainerMerkleTree()); + } ContainerData data = container.getContainerData(); long id = data.getContainerID(); ByteBuffer byteBuffer = ByteBuffer.allocate(Long.BYTES) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java index aef3965dcd4..381348781d4 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java @@ -30,10 +30,11 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.IncrementalContainerReportProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.PipelineReportsProto; import org.apache.hadoop.hdds.security.SecurityConfig; -import org.apache.hadoop.hdds.security.symmetric.SecretKeyVerifierClient; +import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient; import org.apache.hadoop.hdds.security.token.TokenVerifier; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.utils.HddsServerUtil; +import org.apache.hadoop.ozone.container.checksum.DNContainerOperationClient; import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics; import org.apache.hadoop.ozone.container.common.impl.BlockDeletingService; import org.apache.hadoop.ozone.container.common.impl.ContainerSet; @@ -119,6 +120,7 @@ public class OzoneContainer { private final GrpcTlsConfig tlsClientConfig; private final AtomicReference initializingStatus; private final ReplicationServer replicationServer; + private final DNContainerOperationClient dnClient; private DatanodeDetails datanodeDetails; private StateContext context; @@ -141,7 +143,7 @@ enum InitializingStatus { public OzoneContainer( DatanodeDetails datanodeDetails, ConfigurationSource conf, StateContext context, CertificateClient certClient, - SecretKeyVerifierClient secretKeyClient) throws IOException { + SecretKeyClient secretKeyClient) throws IOException { config = conf; this.datanodeDetails = datanodeDetails; this.context = context; @@ -169,6 +171,8 @@ public OzoneContainer( containerSet = new ContainerSet(recoveringContainerTimeout); metadataScanner = null; + dnClient = new DNContainerOperationClient(conf, certClient, secretKeyClient); + metrics = ContainerMetrics.create(conf); handlers = Maps.newHashMap(); @@ -190,7 +194,7 @@ public OzoneContainer( Handler.getHandlerForContainerType( containerType, conf, context.getParent().getDatanodeDetails().getUuidString(), - containerSet, volumeSet, metrics, icrSender)); + containerSet, volumeSet, metrics, icrSender, dnClient)); } SecurityConfig secConf = new SecurityConfig(conf); @@ -284,6 +288,10 @@ public GrpcTlsConfig getTlsClientConfig() { return tlsClientConfig; } + public DNContainerOperationClient getDnContainerOperationClient() { + return dnClient; + } + /** * Build's container map after volume format. */ diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerMerkleTree.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerMerkleTree.java index a93c4f17023..89908620a35 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerMerkleTree.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerMerkleTree.java @@ -38,7 +38,10 @@ import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; -class TestContainerMerkleTree { +/** + * Class to test Container Merkle Tree. + */ +public class TestContainerMerkleTree { private static final long CHUNK_SIZE = (long) new OzoneConfiguration().getStorageSize( ScmConfigKeys.OZONE_SCM_CHUNK_SIZE_KEY, ScmConfigKeys.OZONE_SCM_CHUNK_SIZE_DEFAULT, StorageUnit.BYTES); private static final int BYTES_PER_CHECKSUM = new OzoneClientConfig().getBytesPerChecksum(); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java index bc56141fb08..036574fff30 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java @@ -809,8 +809,7 @@ public void testBlockDeletionTimeout(ContainerTestVersionInfo versionInfo) ContainerMetrics metrics = ContainerMetrics.create(conf); KeyValueHandler keyValueHandler = new KeyValueHandler(conf, datanodeUuid, containerSet, volumeSet, - metrics, c -> { - }); + metrics, c -> { }, null); // set timeout value as 1ns to trigger timeout behavior long timeout = 1; OzoneContainer ozoneContainer = diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java index 1cbd6ee4706..cd45f0e986a 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java @@ -149,7 +149,7 @@ public void testContainerCloseActionWhenFull( handlers.put(containerType, Handler.getHandlerForContainerType(containerType, conf, context.getParent().getDatanodeDetails().getUuidString(), - containerSet, volumeSet, metrics, NO_OP_ICR_SENDER)); + containerSet, volumeSet, metrics, NO_OP_ICR_SENDER, null)); } HddsDispatcher hddsDispatcher = new HddsDispatcher( conf, containerSet, volumeSet, handlers, context, metrics, null); @@ -285,7 +285,7 @@ public void testContainerCloseActionWhenVolumeFull( handlers.put(containerType, Handler.getHandlerForContainerType(containerType, conf, context.getParent().getDatanodeDetails().getUuidString(), - containerSet, volumeSet, metrics, NO_OP_ICR_SENDER)); + containerSet, volumeSet, metrics, NO_OP_ICR_SENDER, null)); } HddsDispatcher hddsDispatcher = new HddsDispatcher( conf, containerSet, volumeSet, handlers, context, metrics, null); @@ -534,7 +534,7 @@ static HddsDispatcher createDispatcher(DatanodeDetails dd, UUID scmId, handlers.put(containerType, Handler.getHandlerForContainerType(containerType, conf, context.getParent().getDatanodeDetails().getUuidString(), - containerSet, volumeSet, metrics, NO_OP_ICR_SENDER)); + containerSet, volumeSet, metrics, NO_OP_ICR_SENDER, null)); } final HddsDispatcher hddsDispatcher = new HddsDispatcher(conf, diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/interfaces/TestHandler.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/interfaces/TestHandler.java index 8f2ad307e82..088af3e0aa2 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/interfaces/TestHandler.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/interfaces/TestHandler.java @@ -71,7 +71,7 @@ public void setup() throws Exception { containerType, conf, context.getParent().getDatanodeDetails().getUuidString(), containerSet, volumeSet, metrics, - TestHddsDispatcher.NO_OP_ICR_SENDER)); + TestHddsDispatcher.NO_OP_ICR_SENDER, null)); } this.dispatcher = new HddsDispatcher( conf, containerSet, volumeSet, handlers, null, metrics, null); diff --git a/hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto b/hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto index 833159c84ec..0fc548a5c0f 100644 --- a/hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto +++ b/hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto @@ -104,6 +104,7 @@ enum Type { StreamInit = 19; StreamWrite = 20; Echo = 21; + ReadContainerMerkleTree = 22; } @@ -210,6 +211,7 @@ message ContainerCommandRequestProto { optional string encodedToken = 23; optional uint32 version = 24; optional EchoRequestProto echo = 25; + optional ReadContainerMerkleTreeRequestProto readContainerMerkleTree = 26; } message ContainerCommandResponseProto { @@ -241,6 +243,7 @@ message ContainerCommandResponseProto { optional GetCommittedBlockLengthResponseProto getCommittedBlockLength = 21; optional EchoResponseProto echo = 22; + optional ReadContainerMerkleTreeResponseProto readContainerMerkleTree = 23; } message ContainerDataProto { @@ -387,6 +390,14 @@ message EchoResponseProto { optional bytes payload = 1; } +message ReadContainerMerkleTreeRequestProto { + optional int64 containerID = 1; +} + +message ReadContainerMerkleTreeResponseProto { + optional int64 containerID = 1; + optional bytes containerMerkleTree = 2; +} // Chunk Operations message ChunkInfo { diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/metrics/TestContainerMetrics.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/metrics/TestContainerMetrics.java index a4a5701f549..834198c9673 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/metrics/TestContainerMetrics.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/metrics/TestContainerMetrics.java @@ -105,7 +105,7 @@ public void testContainerMetrics() throws Exception { Handler.getHandlerForContainerType(containerType, conf, context.getParent().getDatanodeDetails().getUuidString(), containerSet, volumeSet, metrics, - c -> { })); + c -> { }, null)); } HddsDispatcher dispatcher = new HddsDispatcher(conf, containerSet, volumeSet, handlers, context, metrics, null); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java index 630c4d31495..f392e17f452 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java @@ -208,8 +208,7 @@ private HddsDispatcher createDispatcher(DatanodeDetails dd, UUID scmId, Handler.getHandlerForContainerType(containerType, conf, dd.getUuid().toString(), containerSet, volumeSet, metrics, - c -> { - })); + c -> { }, null)); } HddsDispatcher hddsDispatcher = new HddsDispatcher( conf, containerSet, volumeSet, handlers, context, metrics, null); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java index 8044685bb74..a46483e6002 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java @@ -191,7 +191,7 @@ private HddsDispatcher createDispatcher(DatanodeDetails dd, UUID scmId, Handler.getHandlerForContainerType(containerType, conf, dd.getUuid().toString(), containerSet, volumeSet, metrics, - c -> { })); + c -> { }, null)); } HddsDispatcher hddsDispatcher = new HddsDispatcher( conf, containerSet, volumeSet, handlers, context, metrics, diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java new file mode 100644 index 00000000000..6656cdab670 --- /dev/null +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java @@ -0,0 +1,196 @@ +/* + * 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.ozone.dn.checksum; + +import org.apache.commons.io.IOUtils; +import org.apache.hadoop.hdds.HddsConfigKeys; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.scm.ScmConfigKeys; +import org.apache.hadoop.hdds.scm.container.ContainerInfo; +import org.apache.hadoop.ozone.HddsDatanodeService; +import org.apache.hadoop.ozone.MiniOzoneCluster; +import org.apache.hadoop.ozone.client.ObjectStore; +import org.apache.hadoop.ozone.client.OzoneBucket; +import org.apache.hadoop.ozone.client.OzoneClient; +import org.apache.hadoop.ozone.client.OzoneClientFactory; +import org.apache.hadoop.ozone.client.OzoneVolume; +import org.apache.hadoop.ozone.client.io.OzoneOutputStream; +import org.apache.hadoop.ozone.container.checksum.ContainerMerkleTree; +import org.apache.hadoop.ozone.container.checksum.DNContainerOperationClient; +import org.apache.hadoop.ozone.container.common.helpers.ChunkInfo; +import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainer; +import org.apache.hadoop.ozone.container.keyvalue.KeyValueHandler; +import org.apache.ozone.test.GenericTestUtils; +import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.List; +import java.util.UUID; +import java.util.stream.Collectors; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_ENABLED_DEFAULT; +import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_ENABLED_KEY; +import static org.apache.hadoop.ozone.container.checksum.TestContainerMerkleTree.assertTreesSortedAndMatch; +import static org.apache.hadoop.ozone.container.checksum.TestContainerMerkleTree.buildChunk; + +/** + * This class tests container commands for reconciliation. + */ +public class TestContainerCommandReconciliation { + + private static final Logger LOG = LoggerFactory + .getLogger(TestContainerCommandReconciliation.class); + private static MiniOzoneCluster cluster; + private static OzoneClient rpcClient; + private static ObjectStore store; + private static OzoneConfiguration conf; + private static File testDir; + private static OzoneBucket bucket; + private static OzoneVolume volume; + + @BeforeAll + public static void init() throws Exception { + testDir = GenericTestUtils.getTestDir( + TestContainerCommandReconciliation.class.getSimpleName()); + conf = new OzoneConfiguration(); + conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath()); + conf.setInt(ScmConfigKeys.OZONE_SCM_PIPELINE_OWNER_CONTAINER_COUNT, 1); + conf.set(OZONE_METADATA_DIRS, testDir.getAbsolutePath()); + conf.getBoolean(OZONE_SECURITY_ENABLED_KEY, + OZONE_SECURITY_ENABLED_DEFAULT); + cluster = MiniOzoneCluster.newBuilder(conf) + .setNumDatanodes(3) + .build(); + cluster.waitForClusterToBeReady(); + rpcClient = OzoneClientFactory.getRpcClient(conf); + store = rpcClient.getObjectStore(); + } + + @AfterAll + public static void stop() throws IOException { + if (rpcClient != null) { + rpcClient.close(); + } + + if (cluster != null) { + cluster.shutdown(); + } + } + + @Test + public void testReadContainerMerkleTree() throws Exception { + final String volumeName = UUID.randomUUID().toString(); + final String bucketName = UUID.randomUUID().toString(); + final String keyName = UUID.randomUUID().toString(); + byte[] data = "Test content".getBytes(UTF_8); + store.createVolume(volumeName); + volume = store.getVolume(volumeName); + volume.createBucket(bucketName); + bucket = volume.getBucket(bucketName); + // Write Key + try (OzoneOutputStream os = bucket.createKey(keyName, data.length)) { + IOUtils.write(data, os); + } + + // Close container + ContainerInfo container = cluster.getStorageContainerManager() + .getContainerManager().getContainers().get(0); + closeContainer(container); + + //Write Checksum Data + ContainerMerkleTree tree = buildTestTree(); + writeChecksumFileToDatanode(container, tree); + + // Verify all the ContainerMerkle Tree matches + List datanodeDetails = cluster.getHddsDatanodes().stream() + .map(HddsDatanodeService::getDatanodeDetails).collect(Collectors.toList()); + DNContainerOperationClient dnClient = cluster.getHddsDatanodes().get(0).getDatanodeStateMachine() + .getContainer().getDnContainerOperationClient(); + for (DatanodeDetails dn: datanodeDetails) { + ByteString merkleTree = dnClient.readContainerMerkleTree(container.getContainerID(), dn); + ContainerProtos.ContainerChecksumInfo containerChecksumInfo = + ContainerProtos.ContainerChecksumInfo.parseFrom(merkleTree); + assertTreesSortedAndMatch(tree.toProto(), containerChecksumInfo.getContainerMerkleTree()); + } + } + + public static void writeChecksumFileToDatanode(ContainerInfo container, ContainerMerkleTree tree) throws Exception { + // Write Container Merkle Tree + for (HddsDatanodeService dn : cluster.getHddsDatanodes()) { + KeyValueHandler keyValueHandler = + (KeyValueHandler) dn.getDatanodeStateMachine().getContainer().getDispatcher() + .getHandler(ContainerProtos.ContainerType.KeyValueContainer); + KeyValueContainer keyValueContainer = + (KeyValueContainer) dn.getDatanodeStateMachine().getContainer().getController() + .getContainer(container.getContainerID()); + keyValueHandler.getChecksumManager().writeContainerDataTree( + keyValueContainer.getContainerData(), tree); + } + } + + public static void closeContainer(ContainerInfo container) throws Exception { + //Close the container first. + cluster.getStorageContainerManager().getClientProtocolServer().closeContainer(container.getContainerID()); + GenericTestUtils.waitFor(() -> checkContainerState(container), 100, 50000); + } + + private static boolean checkContainerState(ContainerInfo container) { + ContainerInfo containerInfo = null; + try { + containerInfo = cluster.getStorageContainerManager() + .getContainerInfo(container.getContainerID()); + return containerInfo.getState() == HddsProtos.LifeCycleState.CLOSED; + } catch (IOException e) { + LOG.error("Container is in state :" + containerInfo.getState()); + } + return false; + } + + public static ContainerMerkleTree buildTestTree() throws Exception { + final long blockID1 = 1; + final long blockID2 = 2; + final long blockID3 = 3; + ChunkInfo b1c1 = buildChunk(0, ByteBuffer.wrap(new byte[]{1, 2, 3})); + ChunkInfo b1c2 = buildChunk(1, ByteBuffer.wrap(new byte[]{4, 5, 6})); + ChunkInfo b2c1 = buildChunk(0, ByteBuffer.wrap(new byte[]{7, 8, 9})); + ChunkInfo b2c2 = buildChunk(1, ByteBuffer.wrap(new byte[]{12, 11, 10})); + ChunkInfo b3c1 = buildChunk(0, ByteBuffer.wrap(new byte[]{13, 14, 15})); + ChunkInfo b3c2 = buildChunk(1, ByteBuffer.wrap(new byte[]{16, 17, 18})); + + ContainerMerkleTree tree = new ContainerMerkleTree(); + tree.addChunks(blockID1, Arrays.asList(b1c1, b1c2)); + tree.addChunks(blockID2, Arrays.asList(b2c1, b2c2)); + tree.addChunks(blockID3, Arrays.asList(b3c1, b3c2)); + + return tree; + } +} diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java index 5592926bf88..9e7100ff6de 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java @@ -150,8 +150,8 @@ public void loadContainersFromVolumes() throws IOException { containerSet, volumeSet, metrics, - containerReplicaProto -> { - }); + containerReplicaProto -> { }, + null); handler.setClusterID(clusterId); handlers.put(containerType, handler); } diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java index d471c13462f..a856e002c1f 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java @@ -192,8 +192,8 @@ private void initializeReplicationSupervisor( containerSet, volumeSet, metrics, - containerReplicaProto -> { - }); + containerReplicaProto -> { }, + null); handler.setClusterID(UUID.randomUUID().toString()); handlers.put(containerType, handler); } From 737361f860b41c233494a7055573b056691bef2e Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Mon, 15 Jul 2024 21:45:24 +0530 Subject: [PATCH 02/11] HDDS-10376. Fix findbugs. --- .../dn/checksum/TestContainerCommandReconciliation.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java index 6656cdab670..8787172752d 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java @@ -74,8 +74,6 @@ public class TestContainerCommandReconciliation { private static ObjectStore store; private static OzoneConfiguration conf; private static File testDir; - private static OzoneBucket bucket; - private static OzoneVolume volume; @BeforeAll public static void init() throws Exception { @@ -113,9 +111,9 @@ public void testReadContainerMerkleTree() throws Exception { final String keyName = UUID.randomUUID().toString(); byte[] data = "Test content".getBytes(UTF_8); store.createVolume(volumeName); - volume = store.getVolume(volumeName); + OzoneVolume volume = store.getVolume(volumeName); volume.createBucket(bucketName); - bucket = volume.getBucket(bucketName); + OzoneBucket bucket = volume.getBucket(bucketName); // Write Key try (OzoneOutputStream os = bucket.createKey(keyName, data.length)) { IOUtils.write(data, os); @@ -170,7 +168,7 @@ private static boolean checkContainerState(ContainerInfo container) { .getContainerInfo(container.getContainerID()); return containerInfo.getState() == HddsProtos.LifeCycleState.CLOSED; } catch (IOException e) { - LOG.error("Container is in state :" + containerInfo.getState()); + LOG.error("Error when getting the container info", e); } return false; } From 76119bc01df67764ea6b5ad6f8d24abb225c8035 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Tue, 23 Jul 2024 20:39:52 +0530 Subject: [PATCH 03/11] Move TokenHelper and change readMerkleTree to getMerkleTree --- .../ContainerCommandResponseBuilders.java | 12 +++++------ .../scm/storage/ContainerProtocolCalls.java | 18 ++++++++++------ .../apache/hadoop/ozone/audit/DNAction.java | 2 +- .../{ec/reconstruction => }/TokenHelper.java | 2 +- .../ContainerChecksumTreeManager.java | 6 +++--- .../checksum/DNContainerOperationClient.java | 12 +++++------ .../container/common/impl/HddsDispatcher.java | 2 +- .../ECReconstructionCoordinator.java | 3 ++- .../container/keyvalue/KeyValueHandler.java | 16 +++++++------- .../hadoop/ozone/container/package-info.java | 21 +++++++++++++++++++ .../main/proto/DatanodeClientProtocol.proto | 10 ++++----- .../TestContainerCommandReconciliation.java | 4 ++-- 12 files changed, 68 insertions(+), 40 deletions(-) rename hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/{ec/reconstruction => }/TokenHelper.java (98%) create mode 100644 hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/package-info.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java index 280b3d858cd..7461b9954e9 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java @@ -335,15 +335,15 @@ public static ContainerCommandResponseProto getEchoResponse( .build(); } - public static ContainerCommandResponseProto getReadContainerMerkleTreeResponse( - ContainerCommandRequestProto request, byte[] checksumBytes) { + public static ContainerCommandResponseProto getGetContainerMerkleTreeResponse( + ContainerCommandRequestProto request, ByteString checksumByteString) { - ContainerProtos.ReadContainerMerkleTreeResponseProto.Builder containerMerkleTree = - ContainerProtos.ReadContainerMerkleTreeResponseProto.newBuilder() + ContainerProtos.GetContainerMerkleTreeResponseProto.Builder containerMerkleTree = + ContainerProtos.GetContainerMerkleTreeResponseProto.newBuilder() .setContainerID(request.getContainerID()) - .setContainerMerkleTree(ByteString.copyFrom(checksumBytes)); + .setContainerMerkleTree(checksumByteString); return getSuccessResponseBuilder(request) - .setReadContainerMerkleTree(containerMerkleTree).build(); + .setGetContainerMerkleTree(containerMerkleTree).build(); } private ContainerCommandResponseBuilders() { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java index 2866a80be45..6ee48c06b61 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java @@ -723,10 +723,16 @@ public static EchoResponseProto echo(XceiverClientSpi client, String encodedCont return response.getEcho(); } - public static ContainerProtos.ReadContainerMerkleTreeResponseProto readContainerMerkleTree( + /** + * Gets the Container merkle tree for a container from a datanode. + * @param client - client that communicates with the container + * @param containerID - Container Id of the container + * @param encodedContainerID - Encoded token if security is enabled + */ + public static ContainerProtos.GetContainerMerkleTreeResponseProto getContainerMerkleTree( XceiverClientSpi client, long containerID, String encodedContainerID) throws IOException { - ContainerProtos.ReadContainerMerkleTreeRequestProto containerMerkleTreeRequestProto = - ContainerProtos.ReadContainerMerkleTreeRequestProto + ContainerProtos.GetContainerMerkleTreeRequestProto containerMerkleTreeRequestProto = + ContainerProtos.GetContainerMerkleTreeRequestProto .newBuilder() .setContainerID(containerID) .build(); @@ -734,10 +740,10 @@ public static ContainerProtos.ReadContainerMerkleTreeResponseProto readContainer ContainerCommandRequestProto.Builder builder = ContainerCommandRequestProto .newBuilder() - .setCmdType(Type.ReadContainerMerkleTree) + .setCmdType(Type.GetContainerMerkleTree) .setContainerID(containerID) .setDatanodeUuid(id) - .setReadContainerMerkleTree(containerMerkleTreeRequestProto); + .setGetContainerMerkleTree(containerMerkleTreeRequestProto); if (encodedContainerID != null) { builder.setEncodedToken(encodedContainerID); } @@ -748,7 +754,7 @@ public static ContainerProtos.ReadContainerMerkleTreeResponseProto readContainer ContainerCommandRequestProto request = builder.build(); ContainerCommandResponseProto response = client.sendCommand(request, getValidatorList()); - return response.getReadContainerMerkleTree(); + return response.getGetContainerMerkleTree(); } /** diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/audit/DNAction.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/audit/DNAction.java index 6702a988938..e170bf85915 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/audit/DNAction.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/audit/DNAction.java @@ -41,7 +41,7 @@ public enum DNAction implements AuditAction { GET_COMMITTED_BLOCK_LENGTH, STREAM_INIT, ECHO, - READ_CONTAINER_MERKLE_TREE; + GET_CONTAINER_MERKLE_TREE; @Override public String getAction() { diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/TokenHelper.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/TokenHelper.java similarity index 98% rename from hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/TokenHelper.java rename to hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/TokenHelper.java index 2f42a1161f1..801428cc61e 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/TokenHelper.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/TokenHelper.java @@ -15,7 +15,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.hadoop.ozone.container.ec.reconstruction; +package org.apache.hadoop.ozone.container; import org.apache.hadoop.hdds.client.BlockID; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.BlockTokenSecretProto.AccessModeProto; diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java index c54e1af468f..e539d15a12e 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java @@ -16,7 +16,6 @@ */ package org.apache.hadoop.ozone.container.checksum; -import org.apache.commons.io.IOUtils; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; @@ -33,6 +32,7 @@ import com.google.common.util.concurrent.Striped; import org.apache.hadoop.hdds.utils.SimpleStriped; +import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -159,7 +159,7 @@ private void write(KeyValueContainerData data, ContainerProtos.ContainerChecksum } } - public byte[] readChecksumFileAsBytes(KeyValueContainerData data) { + public ByteString readChecksumFileAsBytes(KeyValueContainerData data) { long containerID = data.getContainerID(); Lock readLock = getReadLock(containerID); readLock.lock(); @@ -167,7 +167,7 @@ public byte[] readChecksumFileAsBytes(KeyValueContainerData data) { File checksumFile = getContainerChecksumFile(data); try (FileInputStream inStream = new FileInputStream(checksumFile)) { - return IOUtils.toByteArray(inStream); + return ByteString.readFrom(inStream); } catch (FileNotFoundException ex) { LOG.info("No checksum file currently exists for container {} at the path {}. Returning an empty instance.", containerID, checksumFile, ex); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java index c1cb21f6b78..d42ca897777 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java @@ -37,18 +37,18 @@ import org.apache.hadoop.hdds.utils.HAUtils; import org.apache.hadoop.ozone.OzoneSecurityUtil; import jakarta.annotation.Nonnull; -import org.apache.hadoop.ozone.container.ec.reconstruction.TokenHelper; +import org.apache.hadoop.ozone.container.TokenHelper; import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; -import static org.apache.hadoop.ozone.container.ec.reconstruction.TokenHelper.encode; +import static org.apache.hadoop.ozone.container.TokenHelper.encode; /** * This class wraps necessary container-level rpc calls for container reconcilitaion. - * - ReadContainerMerkleTree + * - GetContainerMerkleTree */ public class DNContainerOperationClient implements AutoCloseable { @@ -86,14 +86,14 @@ public XceiverClientManager getXceiverClientManager() { return xceiverClientManager; } - public ByteString readContainerMerkleTree(long containerId, DatanodeDetails dn) + public ByteString getContainerMerkleTree(long containerId, DatanodeDetails dn) throws IOException { XceiverClientSpi xceiverClient = this.xceiverClientManager.acquireClient(createSingleNodePipeline(dn)); try { String containerToken = encode(tokenHelper.getContainerToken( ContainerID.valueOf(containerId))); - ContainerProtos.ReadContainerMerkleTreeResponseProto response = - ContainerProtocolCalls.readContainerMerkleTree(xceiverClient, + ContainerProtos.GetContainerMerkleTreeResponseProto response = + ContainerProtocolCalls.getContainerMerkleTree(xceiverClient, containerId, containerToken); return response.getContainerMerkleTree(); } finally { diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java index f84673f2fe6..8c9e913c635 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java @@ -815,7 +815,7 @@ private static DNAction getAuditAction(Type cmdType) { case GetCommittedBlockLength : return DNAction.GET_COMMITTED_BLOCK_LENGTH; case StreamInit : return DNAction.STREAM_INIT; case Echo : return DNAction.ECHO; - case ReadContainerMerkleTree : return DNAction.READ_CONTAINER_MERKLE_TREE; + case GetContainerMerkleTree : return DNAction.GET_CONTAINER_MERKLE_TREE; default : LOG.debug("Invalid command type - {}", cmdType); return null; diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java index 8fadd19b67d..5939989c776 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java @@ -47,6 +47,7 @@ import org.apache.hadoop.ozone.client.io.BlockInputStreamFactoryImpl; import org.apache.hadoop.ozone.client.io.ECBlockInputStreamProxy; import org.apache.hadoop.ozone.client.io.ECBlockReconstructedStripeInputStream; +import org.apache.hadoop.ozone.container.TokenHelper; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.statemachine.StateContext; import org.apache.hadoop.security.token.Token; @@ -76,7 +77,7 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; -import static org.apache.hadoop.ozone.container.ec.reconstruction.TokenHelper.encode; +import static org.apache.hadoop.ozone.container.TokenHelper.encode; /** * The Coordinator implements the main flow of reconstructing diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java index d126aa9d489..63f177d2fea 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java @@ -112,7 +112,7 @@ import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getListBlockResponse; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getPutFileResponseSuccess; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getReadChunkResponse; -import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getReadContainerMerkleTreeResponse; +import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getGetContainerMerkleTreeResponse; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getReadContainerResponse; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getSuccessResponse; import static org.apache.hadoop.hdds.scm.protocolPB.ContainerCommandResponseBuilders.getSuccessResponseBuilder; @@ -298,8 +298,8 @@ static ContainerCommandResponseProto dispatchRequest(KeyValueHandler handler, return handler.handleGetCommittedBlockLength(request, kvContainer); case Echo: return handler.handleEcho(request, kvContainer); - case ReadContainerMerkleTree: - return handler.handleReadContainerMerkleTree(request, kvContainer); + case GetContainerMerkleTree: + return handler.handleGetContainerMerkleTree(request, kvContainer); default: return null; } @@ -598,10 +598,10 @@ ContainerCommandResponseProto handleEcho( return getEchoResponse(request); } - ContainerCommandResponseProto handleReadContainerMerkleTree( + ContainerCommandResponseProto handleGetContainerMerkleTree( ContainerCommandRequestProto request, KeyValueContainer kvContainer) { - if (!request.hasReadContainerMerkleTree()) { + if (!request.hasGetContainerMerkleTree()) { if (LOG.isDebugEnabled()) { LOG.debug("Malformed Read Container Merkle tree request. trace ID: {}", request.getTraceID()); @@ -610,8 +610,8 @@ ContainerCommandResponseProto handleReadContainerMerkleTree( } KeyValueContainerData containerData = kvContainer.getContainerData(); - byte[] checksumBytes = checksumManager.readChecksumFileAsBytes(containerData); - return getReadContainerMerkleTreeResponse(request, checksumBytes); + ByteString checksumByteString = checksumManager.readChecksumFileAsBytes(containerData); + return getGetContainerMerkleTreeResponse(request, checksumByteString); } /** @@ -1210,7 +1210,7 @@ public void reconcileContainer(Container container, List pee KeyValueContainerData containerData = (KeyValueContainerData) container.getContainerData(); ByteString containerMerkleTree = - dnClient.readContainerMerkleTree(containerData.getContainerID(), dn); + dnClient.getContainerMerkleTree(containerData.getContainerID(), dn); ContainerProtos.ContainerChecksumInfo containerChecksumInfo = ContainerProtos.ContainerChecksumInfo.parseFrom(containerMerkleTree); LOG.debug("Container Merkle Tree for container {} is {}", diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/package-info.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/package-info.java new file mode 100644 index 00000000000..7ffc29bdc01 --- /dev/null +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/package-info.java @@ -0,0 +1,21 @@ +/** + * 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.ozone.container; +/** + * This package handles container level operations. + */ diff --git a/hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto b/hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto index 0fc548a5c0f..8e58458d524 100644 --- a/hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto +++ b/hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto @@ -104,7 +104,7 @@ enum Type { StreamInit = 19; StreamWrite = 20; Echo = 21; - ReadContainerMerkleTree = 22; + GetContainerMerkleTree = 22; } @@ -211,7 +211,7 @@ message ContainerCommandRequestProto { optional string encodedToken = 23; optional uint32 version = 24; optional EchoRequestProto echo = 25; - optional ReadContainerMerkleTreeRequestProto readContainerMerkleTree = 26; + optional GetContainerMerkleTreeRequestProto getContainerMerkleTree = 26; } message ContainerCommandResponseProto { @@ -243,7 +243,7 @@ message ContainerCommandResponseProto { optional GetCommittedBlockLengthResponseProto getCommittedBlockLength = 21; optional EchoResponseProto echo = 22; - optional ReadContainerMerkleTreeResponseProto readContainerMerkleTree = 23; + optional GetContainerMerkleTreeResponseProto getContainerMerkleTree = 23; } message ContainerDataProto { @@ -390,11 +390,11 @@ message EchoResponseProto { optional bytes payload = 1; } -message ReadContainerMerkleTreeRequestProto { +message GetContainerMerkleTreeRequestProto { optional int64 containerID = 1; } -message ReadContainerMerkleTreeResponseProto { +message GetContainerMerkleTreeResponseProto { optional int64 containerID = 1; optional bytes containerMerkleTree = 2; } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java index 8787172752d..810e40b4da1 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java @@ -105,7 +105,7 @@ public static void stop() throws IOException { } @Test - public void testReadContainerMerkleTree() throws Exception { + public void testGetContainerMerkleTree() throws Exception { final String volumeName = UUID.randomUUID().toString(); final String bucketName = UUID.randomUUID().toString(); final String keyName = UUID.randomUUID().toString(); @@ -134,7 +134,7 @@ public void testReadContainerMerkleTree() throws Exception { DNContainerOperationClient dnClient = cluster.getHddsDatanodes().get(0).getDatanodeStateMachine() .getContainer().getDnContainerOperationClient(); for (DatanodeDetails dn: datanodeDetails) { - ByteString merkleTree = dnClient.readContainerMerkleTree(container.getContainerID(), dn); + ByteString merkleTree = dnClient.getContainerMerkleTree(container.getContainerID(), dn); ContainerProtos.ContainerChecksumInfo containerChecksumInfo = ContainerProtos.ContainerChecksumInfo.parseFrom(merkleTree); assertTreesSortedAndMatch(tree.toProto(), containerChecksumInfo.getContainerMerkleTree()); From 688353fdb01501cf713920f16dfcf7ca21255340 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Thu, 25 Jul 2024 21:15:38 +0530 Subject: [PATCH 04/11] Address review comments. --- .../ContainerCommandResponseBuilders.java | 4 +- .../ContainerChecksumTreeManager.java | 11 +++-- .../checksum/DNContainerOperationClient.java | 11 +++-- .../{ => common/helpers}/TokenHelper.java | 2 +- .../statemachine/DatanodeConfiguration.java | 43 +++++++++++++++++++ .../ECReconstructionCoordinator.java | 4 +- .../container/keyvalue/KeyValueHandler.java | 16 ++++++- .../hadoop/ozone/container/package-info.java | 21 --------- 8 files changed, 76 insertions(+), 36 deletions(-) rename hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/{ => common/helpers}/TokenHelper.java (98%) delete mode 100644 hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/package-info.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java index 7461b9954e9..c8e1eb5ae44 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java @@ -336,12 +336,12 @@ public static ContainerCommandResponseProto getEchoResponse( } public static ContainerCommandResponseProto getGetContainerMerkleTreeResponse( - ContainerCommandRequestProto request, ByteString checksumByteString) { + ContainerCommandRequestProto request, ByteString checksumTree) { ContainerProtos.GetContainerMerkleTreeResponseProto.Builder containerMerkleTree = ContainerProtos.GetContainerMerkleTreeResponseProto.newBuilder() .setContainerID(request.getContainerID()) - .setContainerMerkleTree(checksumByteString); + .setContainerMerkleTree(checksumTree); return getSuccessResponseBuilder(request) .setGetContainerMerkleTree(containerMerkleTree).build(); } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java index e539d15a12e..2b26d38cbb9 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java @@ -159,7 +159,8 @@ private void write(KeyValueContainerData data, ContainerProtos.ContainerChecksum } } - public ByteString readChecksumFileAsBytes(KeyValueContainerData data) { + public ByteString getContainerChecksumInfo(KeyValueContainerData data) + throws IOException { long containerID = data.getContainerID(); Lock readLock = getReadLock(containerID); readLock.lock(); @@ -169,11 +170,13 @@ public ByteString readChecksumFileAsBytes(KeyValueContainerData data) { try (FileInputStream inStream = new FileInputStream(checksumFile)) { return ByteString.readFrom(inStream); } catch (FileNotFoundException ex) { - LOG.info("No checksum file currently exists for container {} at the path {}. Returning an empty instance.", + // TODO: Build the container checksum tree when it doesn't exist. + LOG.error("No checksum file currently exists for container {} at the path {}. Returning an empty instance.", containerID, checksumFile, ex); } catch (IOException ex) { - LOG.info("Error occured when reading checksum file for container {} at the path {}. " + - "Returning an empty instance.", containerID, checksumFile, ex); + throw new IOException("Error occured when reading checksum file for container " + containerID + + " at the path " + checksumFile + ". Returning an empty instance.", ex); + } return null; } finally { diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java index d42ca897777..0a0c55fb784 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java @@ -37,14 +37,15 @@ import org.apache.hadoop.hdds.utils.HAUtils; import org.apache.hadoop.ozone.OzoneSecurityUtil; import jakarta.annotation.Nonnull; -import org.apache.hadoop.ozone.container.TokenHelper; +import org.apache.hadoop.ozone.container.common.helpers.TokenHelper; +import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; -import static org.apache.hadoop.ozone.container.TokenHelper.encode; +import static org.apache.hadoop.ozone.container.common.helpers.TokenHelper.encode; /** * This class wraps necessary container-level rpc calls for container reconcilitaion. @@ -76,10 +77,12 @@ private static XceiverClientManager createClientManager( () -> HAUtils.buildCAX509List(null, conf); trustManager = new ClientTrustManager(remoteCacerts, localCaCerts); } + DatanodeConfiguration dnConf = conf.getObject(DatanodeConfiguration.class); return new XceiverClientManager(conf, new XceiverClientManager.XceiverClientManagerConfigBuilder() - .setMaxCacheSize(100).setStaleThresholdMs(10 * 1000).build(), - trustManager); + .setMaxCacheSize(dnConf.getContainerClientCacheSize()) + .setStaleThresholdMs(dnConf.getContainerClientCacheStaleThreshold()) + .build(), trustManager); } public XceiverClientManager getXceiverClientManager() { diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/TokenHelper.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/TokenHelper.java similarity index 98% rename from hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/TokenHelper.java rename to hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/TokenHelper.java index 801428cc61e..98b4d29635d 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/TokenHelper.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/helpers/TokenHelper.java @@ -15,7 +15,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.hadoop.ozone.container; +package org.apache.hadoop.ozone.container.common.helpers; import org.apache.hadoop.hdds.client.BlockID; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.BlockTokenSecretProto.AccessModeProto; diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java index 28bbb17aa8f..c8e0085d4db 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java @@ -75,6 +75,9 @@ public class DatanodeConfiguration extends ReconfigurableConfig { public static final String CONTAINER_SCHEMA_V3_ENABLED = "hdds.datanode.container.schema.v3.enabled"; public static final String CONTAINER_CHECKSUM_LOCK_STRIPES_KEY = "hdds.datanode.container.checksum.lock.stripes"; + public static final String CONTAINER_CLIENT_CACHE_SIZE = "hdds.datanode.container.client.cache.size"; + public static final String CONTAINER_CLIENT_CACHE_STALE_THRESHOLD = + "hdds.datanode.container.client.cache.stale.threshold"; static final boolean CHUNK_DATA_VALIDATION_CHECK_DEFAULT = false; @@ -111,6 +114,8 @@ public class DatanodeConfiguration extends ReconfigurableConfig { public static final Boolean OZONE_DATANODE_CHECK_EMPTY_CONTAINER_DIR_ON_DELETE_DEFAULT = false; public static final int CONTAINER_CHECKSUM_LOCK_STRIPES_DEFAULT = 127; + public static final int CONTAINER_CLIENT_CACHE_SIZE_DEFAULT = 100; + public static final int CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_DEFAULT = 10000; /** * Number of threads per volume that Datanode will use for chunk read. @@ -567,6 +572,24 @@ public void setWaitOnAllFollowers(boolean val) { ) private int containerChecksumLockStripes = CONTAINER_CHECKSUM_LOCK_STRIPES_DEFAULT; + @Config(key = "container.client.cache.size", + type = ConfigType.INT, + defaultValue = "100", + tags = { DATANODE }, + description = "The maximum number of clients to be cached by the datanode client manager" + ) + private int containerClientCacheSize = CONTAINER_CLIENT_CACHE_SIZE_DEFAULT; + + @Config(key = "container.client.cache.stale.threshold", + type = ConfigType.INT, + defaultValue = "10000", + tags = { DATANODE }, + description = "The stale threshold in ms for a client in cache. After this threshold the client " + + "is evicted from cache." + ) + private int containerClientCacheStaleThreshold = CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_DEFAULT; + + @SuppressWarnings("checkstyle:MethodLength") @PostConstruct public void validate() { if (containerDeleteThreads < 1) { @@ -706,6 +729,18 @@ public void validate() { CONTAINER_CHECKSUM_LOCK_STRIPES_DEFAULT); containerChecksumLockStripes = CONTAINER_CHECKSUM_LOCK_STRIPES_DEFAULT; } + + if (containerClientCacheSize < 1) { + LOG.warn("{} must be at least 1. Defaulting to {}", CONTAINER_CLIENT_CACHE_SIZE, + CONTAINER_CLIENT_CACHE_SIZE_DEFAULT); + containerClientCacheSize = CONTAINER_CLIENT_CACHE_SIZE_DEFAULT; + } + + if (containerClientCacheStaleThreshold < 1) { + LOG.warn("{} must be at least 1. Defaulting to {}", CONTAINER_CLIENT_CACHE_STALE_THRESHOLD, + CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_DEFAULT); + containerClientCacheStaleThreshold = CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_DEFAULT; + } } public void setContainerDeleteThreads(int containerDeleteThreads) { @@ -937,4 +972,12 @@ public void setAutoCompactionSmallSstFileNum(int num) { public int getContainerChecksumLockStripes() { return containerChecksumLockStripes; } + + public int getContainerClientCacheSize() { + return containerClientCacheSize; + } + + public int getContainerClientCacheStaleThreshold() { + return containerClientCacheStaleThreshold; + } } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java index 5939989c776..7e64766b41c 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ec/reconstruction/ECReconstructionCoordinator.java @@ -47,7 +47,7 @@ import org.apache.hadoop.ozone.client.io.BlockInputStreamFactoryImpl; import org.apache.hadoop.ozone.client.io.ECBlockInputStreamProxy; import org.apache.hadoop.ozone.client.io.ECBlockReconstructedStripeInputStream; -import org.apache.hadoop.ozone.container.TokenHelper; +import org.apache.hadoop.ozone.container.common.helpers.TokenHelper; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.statemachine.StateContext; import org.apache.hadoop.security.token.Token; @@ -77,7 +77,7 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; -import static org.apache.hadoop.ozone.container.TokenHelper.encode; +import static org.apache.hadoop.ozone.container.common.helpers.TokenHelper.encode; /** * The Coordinator implements the main flow of reconstructing diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java index 63f177d2fea..c24124a52cd 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java @@ -610,8 +610,19 @@ ContainerCommandResponseProto handleGetContainerMerkleTree( } KeyValueContainerData containerData = kvContainer.getContainerData(); - ByteString checksumByteString = checksumManager.readChecksumFileAsBytes(containerData); - return getGetContainerMerkleTreeResponse(request, checksumByteString); + ByteString checksumTree = null; + try { + checksumTree = checksumManager.getContainerChecksumInfo(containerData); + } catch (IOException ex) { + return ContainerCommandResponseProto.newBuilder() + .setCmdType(request.getCmdType()) + .setTraceID(request.getTraceID()) + .setResult(IO_EXCEPTION) + .setMessage(ex.getMessage()) + .build(); + } + + return getGetContainerMerkleTreeResponse(request, checksumTree); } /** @@ -1206,6 +1217,7 @@ public void deleteContainer(Container container, boolean force) @Override public void reconcileContainer(Container container, List peers) throws IOException { + // TODO Just a deterministic placeholder hash for testing until actual implementation is finished. for (DatanodeDetails dn : peers) { KeyValueContainerData containerData = (KeyValueContainerData) container.getContainerData(); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/package-info.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/package-info.java deleted file mode 100644 index 7ffc29bdc01..00000000000 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/package-info.java +++ /dev/null @@ -1,21 +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.ozone.container; -/** - * This package handles container level operations. - */ From 1737194a62bcc879eddbae9b7057a7e75f02d6c6 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Thu, 25 Jul 2024 23:27:32 +0530 Subject: [PATCH 05/11] remove datanode client --- .../container/common/interfaces/Handler.java | 6 ++--- .../container/keyvalue/KeyValueHandler.java | 23 ------------------- .../container/ozoneimpl/OzoneContainer.java | 10 +------- .../common/TestBlockDeletingService.java | 3 ++- .../common/impl/TestHddsDispatcher.java | 6 ++--- .../common/interfaces/TestHandler.java | 2 +- .../container/server/TestContainerServer.java | 2 +- .../server/TestSecureContainerServer.java | 2 +- .../TestContainerCommandReconciliation.java | 4 ++-- .../debug/container/ContainerCommands.java | 3 +-- .../freon/ClosedContainerReplicator.java | 3 +-- 11 files changed, 15 insertions(+), 49 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java index 78c9b1ade97..8b0f71d035c 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java @@ -30,7 +30,6 @@ import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandResponseProto; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerType; import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException; -import org.apache.hadoop.ozone.container.checksum.DNContainerOperationClient; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics; import org.apache.hadoop.ozone.container.common.impl.ContainerData; @@ -76,13 +75,12 @@ public static Handler getHandlerForContainerType( final ContainerType containerType, final ConfigurationSource config, final String datanodeId, final ContainerSet contSet, final VolumeSet volumeSet, final ContainerMetrics metrics, - IncrementalReportSender icrSender, - DNContainerOperationClient dnClient) { + IncrementalReportSender icrSender) { switch (containerType) { case KeyValueContainer: return new KeyValueHandler(config, datanodeId, contSet, volumeSet, metrics, - icrSender, dnClient); + icrSender); default: throw new IllegalArgumentException("Handler for ContainerType: " + containerType + "doesn't exist."); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java index 8b092ec101f..a8916658beb 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java @@ -63,7 +63,6 @@ import org.apache.hadoop.ozone.common.OzoneChecksumException; import org.apache.hadoop.ozone.common.utils.BufferUtils; import org.apache.hadoop.ozone.container.checksum.ContainerChecksumTreeManager; -import org.apache.hadoop.ozone.container.checksum.DNContainerOperationClient; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.helpers.ChunkInfo; import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics; @@ -147,7 +146,6 @@ public class KeyValueHandler extends Handler { // A striped lock that is held during container creation. private final Striped containerCreationLocks; private final ContainerChecksumTreeManager checksumManager; - private DNContainerOperationClient dnClient; public KeyValueHandler(ConfigurationSource config, String datanodeId, @@ -194,17 +192,6 @@ public KeyValueHandler(ConfigurationSource config, .createByteBufferConversion(isUnsafeByteBufferConversionEnabled); } - public KeyValueHandler(ConfigurationSource config, - String datanodeId, - ContainerSet contSet, - VolumeSet volSet, - ContainerMetrics metrics, - IncrementalReportSender icrSender, - DNContainerOperationClient dnClient) { - this(config, datanodeId, contSet, volSet, metrics, icrSender); - this.dnClient = dnClient; - } - @VisibleForTesting public VolumeChoosingPolicy getVolumeChoosingPolicyForTesting() { return volumeChoosingPolicy; @@ -1231,16 +1218,6 @@ public void deleteContainer(Container container, boolean force) @Override public void reconcileContainer(Container container, List peers) throws IOException { // TODO Just a deterministic placeholder hash for testing until actual implementation is finished. - for (DatanodeDetails dn : peers) { - KeyValueContainerData containerData = - (KeyValueContainerData) container.getContainerData(); - ByteString containerMerkleTree = - dnClient.getContainerMerkleTree(containerData.getContainerID(), dn); - ContainerProtos.ContainerChecksumInfo containerChecksumInfo = - ContainerProtos.ContainerChecksumInfo.parseFrom(containerMerkleTree); - LOG.debug("Container Merkle Tree for container {} is {}", - containerData.getContainerID(), containerChecksumInfo.getContainerMerkleTree()); - } ContainerData data = container.getContainerData(); long id = data.getContainerID(); ByteBuffer byteBuffer = ByteBuffer.allocate(Long.BYTES) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java index 381348781d4..74beacf8106 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java @@ -34,7 +34,6 @@ import org.apache.hadoop.hdds.security.token.TokenVerifier; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.utils.HddsServerUtil; -import org.apache.hadoop.ozone.container.checksum.DNContainerOperationClient; import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics; import org.apache.hadoop.ozone.container.common.impl.BlockDeletingService; import org.apache.hadoop.ozone.container.common.impl.ContainerSet; @@ -120,7 +119,6 @@ public class OzoneContainer { private final GrpcTlsConfig tlsClientConfig; private final AtomicReference initializingStatus; private final ReplicationServer replicationServer; - private final DNContainerOperationClient dnClient; private DatanodeDetails datanodeDetails; private StateContext context; @@ -171,8 +169,6 @@ public OzoneContainer( containerSet = new ContainerSet(recoveringContainerTimeout); metadataScanner = null; - dnClient = new DNContainerOperationClient(conf, certClient, secretKeyClient); - metrics = ContainerMetrics.create(conf); handlers = Maps.newHashMap(); @@ -194,7 +190,7 @@ public OzoneContainer( Handler.getHandlerForContainerType( containerType, conf, context.getParent().getDatanodeDetails().getUuidString(), - containerSet, volumeSet, metrics, icrSender, dnClient)); + containerSet, volumeSet, metrics, icrSender)); } SecurityConfig secConf = new SecurityConfig(conf); @@ -288,10 +284,6 @@ public GrpcTlsConfig getTlsClientConfig() { return tlsClientConfig; } - public DNContainerOperationClient getDnContainerOperationClient() { - return dnClient; - } - /** * Build's container map after volume format. */ diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java index 036574fff30..bc56141fb08 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java @@ -809,7 +809,8 @@ public void testBlockDeletionTimeout(ContainerTestVersionInfo versionInfo) ContainerMetrics metrics = ContainerMetrics.create(conf); KeyValueHandler keyValueHandler = new KeyValueHandler(conf, datanodeUuid, containerSet, volumeSet, - metrics, c -> { }, null); + metrics, c -> { + }); // set timeout value as 1ns to trigger timeout behavior long timeout = 1; OzoneContainer ozoneContainer = diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java index cd45f0e986a..1cbd6ee4706 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestHddsDispatcher.java @@ -149,7 +149,7 @@ public void testContainerCloseActionWhenFull( handlers.put(containerType, Handler.getHandlerForContainerType(containerType, conf, context.getParent().getDatanodeDetails().getUuidString(), - containerSet, volumeSet, metrics, NO_OP_ICR_SENDER, null)); + containerSet, volumeSet, metrics, NO_OP_ICR_SENDER)); } HddsDispatcher hddsDispatcher = new HddsDispatcher( conf, containerSet, volumeSet, handlers, context, metrics, null); @@ -285,7 +285,7 @@ public void testContainerCloseActionWhenVolumeFull( handlers.put(containerType, Handler.getHandlerForContainerType(containerType, conf, context.getParent().getDatanodeDetails().getUuidString(), - containerSet, volumeSet, metrics, NO_OP_ICR_SENDER, null)); + containerSet, volumeSet, metrics, NO_OP_ICR_SENDER)); } HddsDispatcher hddsDispatcher = new HddsDispatcher( conf, containerSet, volumeSet, handlers, context, metrics, null); @@ -534,7 +534,7 @@ static HddsDispatcher createDispatcher(DatanodeDetails dd, UUID scmId, handlers.put(containerType, Handler.getHandlerForContainerType(containerType, conf, context.getParent().getDatanodeDetails().getUuidString(), - containerSet, volumeSet, metrics, NO_OP_ICR_SENDER, null)); + containerSet, volumeSet, metrics, NO_OP_ICR_SENDER)); } final HddsDispatcher hddsDispatcher = new HddsDispatcher(conf, diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/interfaces/TestHandler.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/interfaces/TestHandler.java index 088af3e0aa2..8f2ad307e82 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/interfaces/TestHandler.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/interfaces/TestHandler.java @@ -71,7 +71,7 @@ public void setup() throws Exception { containerType, conf, context.getParent().getDatanodeDetails().getUuidString(), containerSet, volumeSet, metrics, - TestHddsDispatcher.NO_OP_ICR_SENDER, null)); + TestHddsDispatcher.NO_OP_ICR_SENDER)); } this.dispatcher = new HddsDispatcher( conf, containerSet, volumeSet, handlers, null, metrics, null); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java index f392e17f452..5cdcaf24ccf 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java @@ -208,7 +208,7 @@ private HddsDispatcher createDispatcher(DatanodeDetails dd, UUID scmId, Handler.getHandlerForContainerType(containerType, conf, dd.getUuid().toString(), containerSet, volumeSet, metrics, - c -> { }, null)); + c -> { })); } HddsDispatcher hddsDispatcher = new HddsDispatcher( conf, containerSet, volumeSet, handlers, context, metrics, null); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java index a46483e6002..8044685bb74 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestSecureContainerServer.java @@ -191,7 +191,7 @@ private HddsDispatcher createDispatcher(DatanodeDetails dd, UUID scmId, Handler.getHandlerForContainerType(containerType, conf, dd.getUuid().toString(), containerSet, volumeSet, metrics, - c -> { }, null)); + c -> { })); } HddsDispatcher hddsDispatcher = new HddsDispatcher( conf, containerSet, volumeSet, handlers, context, metrics, diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java index 810e40b4da1..2fa62b1bbda 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java @@ -74,6 +74,7 @@ public class TestContainerCommandReconciliation { private static ObjectStore store; private static OzoneConfiguration conf; private static File testDir; + private static DNContainerOperationClient dnClient; @BeforeAll public static void init() throws Exception { @@ -91,6 +92,7 @@ public static void init() throws Exception { cluster.waitForClusterToBeReady(); rpcClient = OzoneClientFactory.getRpcClient(conf); store = rpcClient.getObjectStore(); + dnClient = new DNContainerOperationClient(conf, null, null); } @AfterAll @@ -131,8 +133,6 @@ public void testGetContainerMerkleTree() throws Exception { // Verify all the ContainerMerkle Tree matches List datanodeDetails = cluster.getHddsDatanodes().stream() .map(HddsDatanodeService::getDatanodeDetails).collect(Collectors.toList()); - DNContainerOperationClient dnClient = cluster.getHddsDatanodes().get(0).getDatanodeStateMachine() - .getContainer().getDnContainerOperationClient(); for (DatanodeDetails dn: datanodeDetails) { ByteString merkleTree = dnClient.getContainerMerkleTree(container.getContainerID(), dn); ContainerProtos.ContainerChecksumInfo containerChecksumInfo = diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java index 9e7100ff6de..0071239a2b0 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java @@ -150,8 +150,7 @@ public void loadContainersFromVolumes() throws IOException { containerSet, volumeSet, metrics, - containerReplicaProto -> { }, - null); + containerReplicaProto -> { }); handler.setClusterID(clusterId); handlers.put(containerType, handler); } diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java index a856e002c1f..67cebee869e 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java @@ -192,8 +192,7 @@ private void initializeReplicationSupervisor( containerSet, volumeSet, metrics, - containerReplicaProto -> { }, - null); + containerReplicaProto -> { }); handler.setClusterID(UUID.randomUUID().toString()); handlers.put(containerType, handler); } From 16a084a89c17c37bcf397f2da931d2bdd475902c Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Sat, 27 Jul 2024 18:46:32 +0530 Subject: [PATCH 06/11] Address Review Comments. --- .../container/checksum/ContainerChecksumTreeManager.java | 2 +- .../hadoop/ozone/container/common/interfaces/Handler.java | 1 - .../hadoop/ozone/container/ozoneimpl/OzoneContainer.java | 4 ++-- .../hadoop/ozone/container/server/TestContainerServer.java | 3 ++- .../dn/checksum/TestContainerCommandReconciliation.java | 5 ++++- .../hadoop/ozone/debug/container/ContainerCommands.java | 3 ++- .../apache/hadoop/ozone/freon/ClosedContainerReplicator.java | 3 ++- 7 files changed, 13 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java index a210f2a5ed6..fc29c20c8d6 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java @@ -184,7 +184,7 @@ public ByteString getContainerChecksumInfo(KeyValueContainerData data) containerID, checksumFile, ex); } catch (IOException ex) { throw new IOException("Error occured when reading checksum file for container " + containerID + - " at the path " + checksumFile + ". Returning an empty instance.", ex); + " at the path " + checksumFile, ex); } return null; diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java index 8b0f71d035c..179274f2c02 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java @@ -70,7 +70,6 @@ protected Handler(ConfigurationSource config, String datanodeId, this.icrSender = icrSender; } - @SuppressWarnings("checkstyle:ParameterNumber") public static Handler getHandlerForContainerType( final ContainerType containerType, final ConfigurationSource config, final String datanodeId, final ContainerSet contSet, diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java index 74beacf8106..aef3965dcd4 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java @@ -30,7 +30,7 @@ import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.IncrementalContainerReportProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.PipelineReportsProto; import org.apache.hadoop.hdds.security.SecurityConfig; -import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient; +import org.apache.hadoop.hdds.security.symmetric.SecretKeyVerifierClient; import org.apache.hadoop.hdds.security.token.TokenVerifier; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.utils.HddsServerUtil; @@ -141,7 +141,7 @@ enum InitializingStatus { public OzoneContainer( DatanodeDetails datanodeDetails, ConfigurationSource conf, StateContext context, CertificateClient certClient, - SecretKeyClient secretKeyClient) throws IOException { + SecretKeyVerifierClient secretKeyClient) throws IOException { config = conf; this.datanodeDetails = datanodeDetails; this.context = context; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java index 5cdcaf24ccf..630c4d31495 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/server/TestContainerServer.java @@ -208,7 +208,8 @@ private HddsDispatcher createDispatcher(DatanodeDetails dd, UUID scmId, Handler.getHandlerForContainerType(containerType, conf, dd.getUuid().toString(), containerSet, volumeSet, metrics, - c -> { })); + c -> { + })); } HddsDispatcher hddsDispatcher = new HddsDispatcher( conf, containerSet, volumeSet, handlers, context, metrics, null); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java index 2fa62b1bbda..2c61d3ce725 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/checksum/TestContainerCommandReconciliation.java @@ -44,6 +44,7 @@ import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -73,9 +74,11 @@ public class TestContainerCommandReconciliation { private static OzoneClient rpcClient; private static ObjectStore store; private static OzoneConfiguration conf; - private static File testDir; private static DNContainerOperationClient dnClient; + @TempDir + private static File testDir; + @BeforeAll public static void init() throws Exception { testDir = GenericTestUtils.getTestDir( diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java index 0071239a2b0..5592926bf88 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/container/ContainerCommands.java @@ -150,7 +150,8 @@ public void loadContainersFromVolumes() throws IOException { containerSet, volumeSet, metrics, - containerReplicaProto -> { }); + containerReplicaProto -> { + }); handler.setClusterID(clusterId); handlers.put(containerType, handler); } diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java index 67cebee869e..d471c13462f 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/ClosedContainerReplicator.java @@ -192,7 +192,8 @@ private void initializeReplicationSupervisor( containerSet, volumeSet, metrics, - containerReplicaProto -> { }); + containerReplicaProto -> { + }); handler.setClusterID(UUID.randomUUID().toString()); handlers.put(containerType, handler); } From 46ddedb1043fa7c1d553b339dbab27f78a57675b Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Tue, 30 Jul 2024 00:29:10 +0530 Subject: [PATCH 07/11] Unregister ContainerMerkleTreeMetrics. --- .../ozone/container/checksum/ContainerMerkleTreeMetrics.java | 2 +- .../hadoop/ozone/container/ozoneimpl/OzoneContainer.java | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeMetrics.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeMetrics.java index a288e15f6bd..5bcf2bc04e1 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeMetrics.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeMetrics.java @@ -35,7 +35,7 @@ public static ContainerMerkleTreeMetrics create() { new ContainerMerkleTreeMetrics()); } - public void unregister() { + public static void unregister() { MetricsSystem ms = DefaultMetricsSystem.instance(); ms.unregisterSource(METRICS_SOURCE_NAME); } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java index aef3965dcd4..bb25292c88b 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/OzoneContainer.java @@ -34,6 +34,7 @@ import org.apache.hadoop.hdds.security.token.TokenVerifier; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.utils.HddsServerUtil; +import org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeMetrics; import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics; import org.apache.hadoop.ozone.container.common.impl.BlockDeletingService; import org.apache.hadoop.ozone.container.common.impl.ContainerSet; @@ -494,6 +495,8 @@ public void stop() { blockDeletingService.shutdown(); recoveringContainerScrubbingService.shutdown(); ContainerMetrics.remove(); + // TODO: To properly shut down ContainerMerkleTreeMetrics + ContainerMerkleTreeMetrics.unregister(); } public void handleVolumeFailures() { From e3afa53a9513194e894ba71fc458759d9c8e48c0 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Mon, 5 Aug 2024 12:14:14 -0700 Subject: [PATCH 08/11] Add test for container token. --- .../ContainerChecksumTreeManager.java | 5 +- .../checksum/DNContainerOperationClient.java | 4 +- .../statemachine/DatanodeConfiguration.java | 11 ++-- .../ozoneimpl/TestOzoneContainerWithTLS.java | 62 +++++++++++++++++++ 4 files changed, 73 insertions(+), 9 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java index fc29c20c8d6..0e06e8c239a 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/ContainerChecksumTreeManager.java @@ -180,14 +180,13 @@ public ByteString getContainerChecksumInfo(KeyValueContainerData data) return ByteString.readFrom(inStream); } catch (FileNotFoundException ex) { // TODO: Build the container checksum tree when it doesn't exist. - LOG.error("No checksum file currently exists for container {} at the path {}. Returning an empty instance.", + LOG.debug("No checksum file currently exists for container {} at the path {}. Returning an empty instance.", containerID, checksumFile, ex); } catch (IOException ex) { throw new IOException("Error occured when reading checksum file for container " + containerID + " at the path " + checksumFile, ex); - } - return null; + return ByteString.EMPTY; } finally { readLock.unlock(); } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java index 0a0c55fb784..bdf75763e04 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/checksum/DNContainerOperationClient.java @@ -48,7 +48,7 @@ import static org.apache.hadoop.ozone.container.common.helpers.TokenHelper.encode; /** - * This class wraps necessary container-level rpc calls for container reconcilitaion. + * This class wraps necessary container-level rpc calls for container reconciliation. * - GetContainerMerkleTree */ public class DNContainerOperationClient implements AutoCloseable { @@ -104,7 +104,7 @@ public ByteString getContainerMerkleTree(long containerId, DatanodeDetails dn) } } - private Pipeline createSingleNodePipeline(DatanodeDetails dn) { + public static Pipeline createSingleNodePipeline(DatanodeDetails dn) { return Pipeline.newBuilder() .setNodes(ImmutableList.of(dn)) .setId(PipelineID.valueOf(dn.getUuid())) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java index c8e0085d4db..16cacd6fae8 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java @@ -115,7 +115,8 @@ public class DatanodeConfiguration extends ReconfigurableConfig { OZONE_DATANODE_CHECK_EMPTY_CONTAINER_DIR_ON_DELETE_DEFAULT = false; public static final int CONTAINER_CHECKSUM_LOCK_STRIPES_DEFAULT = 127; public static final int CONTAINER_CLIENT_CACHE_SIZE_DEFAULT = 100; - public static final int CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_DEFAULT = 10000; + public static final int + CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_MILLISECONDS_DEFAULT = 10000; /** * Number of threads per volume that Datanode will use for chunk read. @@ -587,7 +588,8 @@ public void setWaitOnAllFollowers(boolean val) { description = "The stale threshold in ms for a client in cache. After this threshold the client " + "is evicted from cache." ) - private int containerClientCacheStaleThreshold = CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_DEFAULT; + private int containerClientCacheStaleThreshold = + CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_MILLISECONDS_DEFAULT; @SuppressWarnings("checkstyle:MethodLength") @PostConstruct @@ -738,8 +740,9 @@ public void validate() { if (containerClientCacheStaleThreshold < 1) { LOG.warn("{} must be at least 1. Defaulting to {}", CONTAINER_CLIENT_CACHE_STALE_THRESHOLD, - CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_DEFAULT); - containerClientCacheStaleThreshold = CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_DEFAULT; + CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_MILLISECONDS_DEFAULT); + containerClientCacheStaleThreshold = + CONTAINER_CLIENT_CACHE_STALE_THRESHOLD_MILLISECONDS_DEFAULT; } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java index a1e8e1781f5..b713b709a73 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java @@ -27,6 +27,7 @@ import org.apache.hadoop.hdds.scm.XceiverClientManager; import org.apache.hadoop.hdds.scm.XceiverClientManager.ScmClientConfig; import org.apache.hadoop.hdds.scm.client.ClientTrustManager; +import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls; import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient; import org.apache.hadoop.hdds.security.token.ContainerTokenIdentifier; import org.apache.hadoop.hdds.security.token.ContainerTokenSecretManager; @@ -36,6 +37,7 @@ import org.apache.hadoop.hdds.scm.XceiverClientGrpc; import org.apache.hadoop.hdds.scm.XceiverClientSpi; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; +import org.apache.hadoop.ozone.container.checksum.DNContainerOperationClient; import org.apache.hadoop.ozone.container.common.ContainerTestUtils; import org.apache.hadoop.ozone.container.common.statemachine.StateContext; import org.apache.hadoop.ozone.container.common.utils.StorageVolumeUtil; @@ -44,6 +46,7 @@ import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.security.token.Token; import org.apache.ozone.test.GenericTestUtils.LogCapturer; +import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Timeout; @@ -82,6 +85,7 @@ import static org.apache.hadoop.ozone.container.ContainerTestHelper.getCloseContainer; import static org.apache.hadoop.ozone.container.ContainerTestHelper.getCreateContainerSecureRequest; import static org.apache.hadoop.ozone.container.ContainerTestHelper.getTestContainerID; +import static org.apache.hadoop.ozone.container.checksum.DNContainerOperationClient.createSingleNodePipeline; import static org.apache.hadoop.ozone.container.replication.CopyContainerCompression.NO_COMPRESSION; import static org.apache.ozone.test.GenericTestUtils.LogCapturer.captureLogs; import static org.apache.ozone.test.GenericTestUtils.setLogLevel; @@ -224,6 +228,64 @@ public void downloadContainer(boolean containerTokenEnabled) } } + @ParameterizedTest(name = "Container token enabled: {0}") + @ValueSource(booleans = {false, true}) + public void testDNContainerOperationClient(boolean containerTokenEnabled) + throws Exception { + conf.setBoolean(HddsConfigKeys.HDDS_CONTAINER_TOKEN_ENABLED, + containerTokenEnabled); + OzoneContainer container = createAndStartOzoneContainerInstance(); + DNContainerOperationClient dnClient = + new DNContainerOperationClient(conf, caClient, keyClient); + ScmClientConfig scmClientConf = conf.getObject(ScmClientConfig.class); + XceiverClientManager clientManager = + new XceiverClientManager(conf, scmClientConf, aClientTrustManager()); + XceiverClientSpi client = null; + try { + client = clientManager.acquireClient(pipeline); + long containerId = createAndCloseContainer(client, containerTokenEnabled); + ByteString containerMerkleTree = + dnClient.getContainerMerkleTree(containerId, dn); + // Getting container merkle tree with valid container token + assertEquals(containerMerkleTree, ByteString.EMPTY); + + // Getting container merkle tree with invalid container token + if (containerTokenEnabled) { + assertThrows(IOException.class, () -> + getContainerMerkleTree(containerId, dn, "invalidToken", dnClient)); + } else { + containerMerkleTree = getContainerMerkleTree(containerId, dn, "invalidToken", dnClient); + assertEquals(containerMerkleTree, ByteString.EMPTY); + } + } finally { + if (container != null) { + container.stop(); + } + if (client != null) { + clientManager.releaseClient(client, true); + } + if (dnClient != null) { + dnClient.close(); + } + } + } + + public ByteString getContainerMerkleTree(long containerId, + DatanodeDetails datanodeDetails, + String invalidContainerToken, + DNContainerOperationClient dnClient) throws IOException { + XceiverClientSpi xceiverClient = dnClient.getXceiverClientManager() + .acquireClient(createSingleNodePipeline(datanodeDetails)); + try { + ContainerProtos.GetContainerMerkleTreeResponseProto response = + ContainerProtocolCalls.getContainerMerkleTree(xceiverClient, + containerId, invalidContainerToken); + return response.getContainerMerkleTree(); + } finally { + dnClient.getXceiverClientManager().releaseClient(xceiverClient, false); + } + } + @Test public void testLongLivingClientWithCertRenews() throws Exception { LogCapturer logs = captureLogs(getLogger(ClientTrustManager.class)); From 6d557771cc2a6e841c81411c410a9b76624c8ac7 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Mon, 5 Aug 2024 21:12:37 -0700 Subject: [PATCH 09/11] Updates test as per review comments. --- .../ozoneimpl/TestOzoneContainerWithTLS.java | 72 +++++++++---------- 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java index b713b709a73..e07d038a943 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java @@ -27,7 +27,9 @@ import org.apache.hadoop.hdds.scm.XceiverClientManager; import org.apache.hadoop.hdds.scm.XceiverClientManager.ScmClientConfig; import org.apache.hadoop.hdds.scm.client.ClientTrustManager; +import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls; +import org.apache.hadoop.hdds.security.SecurityConfig; import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient; import org.apache.hadoop.hdds.security.token.ContainerTokenIdentifier; import org.apache.hadoop.hdds.security.token.ContainerTokenSecretManager; @@ -39,6 +41,7 @@ import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.ozone.container.checksum.DNContainerOperationClient; import org.apache.hadoop.ozone.container.common.ContainerTestUtils; +import org.apache.hadoop.ozone.container.common.helpers.TokenHelper; import org.apache.hadoop.ozone.container.common.statemachine.StateContext; import org.apache.hadoop.ozone.container.common.utils.StorageVolumeUtil; import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; @@ -85,7 +88,7 @@ import static org.apache.hadoop.ozone.container.ContainerTestHelper.getCloseContainer; import static org.apache.hadoop.ozone.container.ContainerTestHelper.getCreateContainerSecureRequest; import static org.apache.hadoop.ozone.container.ContainerTestHelper.getTestContainerID; -import static org.apache.hadoop.ozone.container.checksum.DNContainerOperationClient.createSingleNodePipeline; +import static org.apache.hadoop.ozone.container.common.helpers.TokenHelper.encode; import static org.apache.hadoop.ozone.container.replication.CopyContainerCompression.NO_COMPRESSION; import static org.apache.ozone.test.GenericTestUtils.LogCapturer.captureLogs; import static org.apache.ozone.test.GenericTestUtils.setLogLevel; @@ -235,54 +238,45 @@ public void testDNContainerOperationClient(boolean containerTokenEnabled) conf.setBoolean(HddsConfigKeys.HDDS_CONTAINER_TOKEN_ENABLED, containerTokenEnabled); OzoneContainer container = createAndStartOzoneContainerInstance(); - DNContainerOperationClient dnClient = - new DNContainerOperationClient(conf, caClient, keyClient); - ScmClientConfig scmClientConf = conf.getObject(ScmClientConfig.class); - XceiverClientManager clientManager = - new XceiverClientManager(conf, scmClientConf, aClientTrustManager()); - XceiverClientSpi client = null; - try { - client = clientManager.acquireClient(pipeline); + try (DNContainerOperationClient dnClient = new DNContainerOperationClient( + conf, caClient, keyClient)) { + XceiverClientSpi client = + dnClient.getXceiverClientManager().acquireClient(pipeline); long containerId = createAndCloseContainer(client, containerTokenEnabled); - ByteString containerMerkleTree = - dnClient.getContainerMerkleTree(containerId, dn); - // Getting container merkle tree with valid container token - assertEquals(containerMerkleTree, ByteString.EMPTY); - - // Getting container merkle tree with invalid container token - if (containerTokenEnabled) { - assertThrows(IOException.class, () -> - getContainerMerkleTree(containerId, dn, "invalidToken", dnClient)); - } else { - containerMerkleTree = getContainerMerkleTree(containerId, dn, "invalidToken", dnClient); - assertEquals(containerMerkleTree, ByteString.EMPTY); - } + dnClient.getContainerMerkleTree(containerId, dn); } finally { if (container != null) { container.stop(); } - if (client != null) { - clientManager.releaseClient(client, true); - } - if (dnClient != null) { - dnClient.close(); - } } } - public ByteString getContainerMerkleTree(long containerId, - DatanodeDetails datanodeDetails, - String invalidContainerToken, - DNContainerOperationClient dnClient) throws IOException { - XceiverClientSpi xceiverClient = dnClient.getXceiverClientManager() - .acquireClient(createSingleNodePipeline(datanodeDetails)); - try { + @Test + public void testContainerMerkleTree() throws IOException { + conf.setBoolean(HddsConfigKeys.HDDS_CONTAINER_TOKEN_ENABLED, true); + OzoneContainer container = createAndStartOzoneContainerInstance(); + ScmClientConfig scmClientConf = conf.getObject(ScmClientConfig.class); + + try (XceiverClientManager clientManager = + new XceiverClientManager(conf, scmClientConf, aClientTrustManager())) { + XceiverClientSpi client = clientManager.acquireClient(pipeline); + long containerId = createAndCloseContainer(client, true); + TokenHelper tokenHelper = new TokenHelper(new SecurityConfig(conf), keyClient); + String containerToken = encode(tokenHelper.getContainerToken( + ContainerID.valueOf(containerId))); ContainerProtos.GetContainerMerkleTreeResponseProto response = - ContainerProtocolCalls.getContainerMerkleTree(xceiverClient, - containerId, invalidContainerToken); - return response.getContainerMerkleTree(); + ContainerProtocolCalls.getContainerMerkleTree(client, + containerId, containerToken); + // Getting container merkle tree with valid container token + assertEquals(response.getContainerMerkleTree(), ByteString.EMPTY); + + // Getting container merkle tree with invalid container token + assertThrows(IOException.class, () -> ContainerProtocolCalls.getContainerMerkleTree(client, + containerId, "invalidContainerToken")); } finally { - dnClient.getXceiverClientManager().releaseClient(xceiverClient, false); + if (container != null) { + container.stop(); + } } } From afac35aa8d38be1c209e7c25f2c07af97b6cdd2d Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Mon, 5 Aug 2024 22:58:54 -0700 Subject: [PATCH 10/11] Close and release clients for test. --- .../ozoneimpl/TestOzoneContainerWithTLS.java | 34 +++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java index e07d038a943..5be274d2399 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java @@ -35,6 +35,7 @@ import org.apache.hadoop.hdds.security.token.ContainerTokenSecretManager; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClientTestImpl; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.utils.IOUtils; import org.apache.hadoop.ozone.client.SecretKeyTestClient; import org.apache.hadoop.hdds.scm.XceiverClientGrpc; import org.apache.hadoop.hdds.scm.XceiverClientSpi; @@ -228,6 +229,7 @@ public void downloadContainer(boolean containerTokenEnabled) if (client != null) { clientManager.releaseClient(client, true); } + IOUtils.closeQuietly(clientManager); } } @@ -238,16 +240,21 @@ public void testDNContainerOperationClient(boolean containerTokenEnabled) conf.setBoolean(HddsConfigKeys.HDDS_CONTAINER_TOKEN_ENABLED, containerTokenEnabled); OzoneContainer container = createAndStartOzoneContainerInstance(); - try (DNContainerOperationClient dnClient = new DNContainerOperationClient( - conf, caClient, keyClient)) { - XceiverClientSpi client = - dnClient.getXceiverClientManager().acquireClient(pipeline); + DNContainerOperationClient dnClient = null; + XceiverClientSpi client = null; + try { + dnClient = new DNContainerOperationClient(conf, caClient, keyClient); + client = dnClient.getXceiverClientManager().acquireClient(pipeline); long containerId = createAndCloseContainer(client, containerTokenEnabled); dnClient.getContainerMerkleTree(containerId, dn); } finally { if (container != null) { container.stop(); } + if (client != null) { + dnClient.getXceiverClientManager().releaseClient(client, true); + } + IOUtils.closeQuietly(dnClient); } } @@ -256,10 +263,11 @@ public void testContainerMerkleTree() throws IOException { conf.setBoolean(HddsConfigKeys.HDDS_CONTAINER_TOKEN_ENABLED, true); OzoneContainer container = createAndStartOzoneContainerInstance(); ScmClientConfig scmClientConf = conf.getObject(ScmClientConfig.class); - - try (XceiverClientManager clientManager = - new XceiverClientManager(conf, scmClientConf, aClientTrustManager())) { - XceiverClientSpi client = clientManager.acquireClient(pipeline); + XceiverClientManager clientManager = + new XceiverClientManager(conf, scmClientConf, aClientTrustManager()); + XceiverClientSpi client = null; + try { + client = clientManager.acquireClient(pipeline); long containerId = createAndCloseContainer(client, true); TokenHelper tokenHelper = new TokenHelper(new SecurityConfig(conf), keyClient); String containerToken = encode(tokenHelper.getContainerToken( @@ -271,12 +279,17 @@ public void testContainerMerkleTree() throws IOException { assertEquals(response.getContainerMerkleTree(), ByteString.EMPTY); // Getting container merkle tree with invalid container token - assertThrows(IOException.class, () -> ContainerProtocolCalls.getContainerMerkleTree(client, - containerId, "invalidContainerToken")); + XceiverClientSpi finalClient = client; + assertThrows(IOException.class, () -> ContainerProtocolCalls.getContainerMerkleTree( + finalClient, containerId, "invalidContainerToken")); } finally { if (container != null) { container.stop(); } + if (client != null) { + clientManager.releaseClient(client, true); + } + IOUtils.closeQuietly(clientManager); } } @@ -342,6 +355,7 @@ public void testLongLivingClientWithCertRenews() throws Exception { if (container != null) { container.stop(); } + IOUtils.closeQuietly(clientManager); } } From 7e57538608f79e130f580ee48c4468f66afac78c Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Tue, 6 Aug 2024 10:28:42 -0700 Subject: [PATCH 11/11] Update test with proper client release. --- .../ozoneimpl/TestOzoneContainerWithTLS.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java index 5be274d2399..fef69f9bf65 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestOzoneContainerWithTLS.java @@ -28,6 +28,7 @@ import org.apache.hadoop.hdds.scm.XceiverClientManager.ScmClientConfig; import org.apache.hadoop.hdds.scm.client.ClientTrustManager; import org.apache.hadoop.hdds.scm.container.ContainerID; +import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException; import org.apache.hadoop.hdds.scm.storage.ContainerProtocolCalls; import org.apache.hadoop.hdds.security.SecurityConfig; import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient; @@ -240,11 +241,13 @@ public void testDNContainerOperationClient(boolean containerTokenEnabled) conf.setBoolean(HddsConfigKeys.HDDS_CONTAINER_TOKEN_ENABLED, containerTokenEnabled); OzoneContainer container = createAndStartOzoneContainerInstance(); - DNContainerOperationClient dnClient = null; + ScmClientConfig scmClientConf = conf.getObject(ScmClientConfig.class); + XceiverClientManager clientManager = + new XceiverClientManager(conf, scmClientConf, aClientTrustManager()); XceiverClientSpi client = null; - try { - dnClient = new DNContainerOperationClient(conf, caClient, keyClient); - client = dnClient.getXceiverClientManager().acquireClient(pipeline); + try (DNContainerOperationClient dnClient = + new DNContainerOperationClient(conf, caClient, keyClient)) { + client = clientManager.acquireClient(pipeline); long containerId = createAndCloseContainer(client, containerTokenEnabled); dnClient.getContainerMerkleTree(containerId, dn); } finally { @@ -252,14 +255,14 @@ public void testDNContainerOperationClient(boolean containerTokenEnabled) container.stop(); } if (client != null) { - dnClient.getXceiverClientManager().releaseClient(client, true); + clientManager.releaseClient(client, true); } - IOUtils.closeQuietly(dnClient); + IOUtils.closeQuietly(clientManager); } } @Test - public void testContainerMerkleTree() throws IOException { + public void testGetContainerMerkleTree() throws IOException { conf.setBoolean(HddsConfigKeys.HDDS_CONTAINER_TOKEN_ENABLED, true); OzoneContainer container = createAndStartOzoneContainerInstance(); ScmClientConfig scmClientConf = conf.getObject(ScmClientConfig.class); @@ -280,8 +283,10 @@ public void testContainerMerkleTree() throws IOException { // Getting container merkle tree with invalid container token XceiverClientSpi finalClient = client; - assertThrows(IOException.class, () -> ContainerProtocolCalls.getContainerMerkleTree( + StorageContainerException exception = assertThrows(StorageContainerException.class, + () -> ContainerProtocolCalls.getContainerMerkleTree( finalClient, containerId, "invalidContainerToken")); + assertEquals(ContainerProtos.Result.BLOCK_TOKEN_VERIFICATION_FAILED, exception.getResult()); } finally { if (container != null) { container.stop();