From 923ca715064775f1f5907438bc4f3ee33fccd934 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Wed, 11 Jun 2025 14:40:16 +0530 Subject: [PATCH 01/10] HDDS-12824. Optimize container checksum read during datanode startup --- .../org/apache/hadoop/ozone/OzoneConsts.java | 2 + .../ContainerChecksumTreeManager.java | 5 ++- .../keyvalue/KeyValueContainerData.java | 5 +++ .../container/keyvalue/KeyValueHandler.java | 12 +++++- .../keyvalue/TarContainerPacker.java | 21 +++++++--- .../helpers/KeyValueContainerUtil.java | 16 +++++++- .../TestContainerChecksumTreeManager.java | 3 +- ...tainerReconciliationWithMockDatanodes.java | 9 ++++- .../keyvalue/TestKeyValueHandler.java | 10 +++++ .../keyvalue/TestTarContainerPacker.java | 40 ++++++++++++++++--- .../impl/TestFilePerBlockStrategy.java | 25 +++++++++--- ...groundContainerDataScannerIntegration.java | 9 +++++ ...DemandContainerDataScannerIntegration.java | 9 +++++ 13 files changed, 140 insertions(+), 26 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index 6bee926336b8..485a690bae20 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -88,6 +88,7 @@ public final class OzoneConsts { public static final Path ROOT_PATH = Paths.get(OZONE_ROOT); public static final String CONTAINER_EXTENSION = ".container"; + public static final String CONTAINER_DATA_CHECKSUM_EXTENSION = ".tree"; public static final String CONTAINER_META_PATH = "metadata"; public static final String CONTAINER_TEMPORARY_CHUNK_PREFIX = "tmp"; public static final String CONTAINER_CHUNK_NAME_DELIMITER = "."; @@ -141,6 +142,7 @@ public final class OzoneConsts { public static final String CONTAINER_BYTES_USED = "#BYTESUSED"; public static final String PENDING_DELETE_BLOCK_COUNT = "#PENDINGDELETEBLOCKCOUNT"; + public static final String CONTAINER_DATA_CHECKSUM = "#DATACHECKSUM"; /** * OM LevelDB prefixes. 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 f32f1debcdc5..0c294000f6b2 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 @@ -19,6 +19,7 @@ import static java.nio.file.StandardCopyOption.ATOMIC_MOVE; import static org.apache.hadoop.hdds.HddsUtils.checksumToString; +import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_DATA_CHECKSUM_EXTENSION; import static org.apache.hadoop.ozone.util.MetricUtil.captureLatencyNs; import com.google.common.annotations.VisibleForTesting; @@ -316,12 +317,12 @@ public static boolean hasContainerChecksumFile(ContainerData data) { */ @VisibleForTesting public static File getContainerChecksumFile(ContainerData data) { - return new File(data.getMetadataPath(), data.getContainerID() + ".tree"); + return new File(data.getMetadataPath(), data.getContainerID() + CONTAINER_DATA_CHECKSUM_EXTENSION); } @VisibleForTesting public static File getTmpContainerChecksumFile(ContainerData data) { - return new File(data.getMetadataPath(), data.getContainerID() + ".tree.tmp"); + return new File(data.getMetadataPath(), data.getContainerID() + CONTAINER_DATA_CHECKSUM_EXTENSION + ".tmp"); } private Lock getLock(long containerID) { diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerData.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerData.java index 4628c3dca1bc..a13017b31c16 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerData.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerData.java @@ -22,6 +22,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.BLOCK_COUNT; import static org.apache.hadoop.ozone.OzoneConsts.CHUNKS_PATH; import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_BYTES_USED; +import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_DATA_CHECKSUM; import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_DB_TYPE; import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_DB_TYPE_ROCKSDB; import static org.apache.hadoop.ozone.OzoneConsts.DELETE_TRANSACTION_KEY; @@ -409,6 +410,10 @@ public String getPendingDeleteBlockCountKey() { return formatKey(PENDING_DELETE_BLOCK_COUNT); } + public String getContainerDataChecksumKey() { + return formatKey(CONTAINER_DATA_CHECKSUM); + } + public String getDeletingBlockKeyPrefix() { return formatKey(DELETING_KEY_PREFIX); } 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 b956e6f50081..972c6c8dab24 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 @@ -1409,7 +1409,7 @@ private void updateContainerChecksumFromMetadataIfNeeded(Container container) { * This method does not send an ICR with the updated checksum info. * @param container - Container for which the container merkle tree needs to be updated. */ - private ContainerProtos.ContainerChecksumInfo updateAndGetContainerChecksumFromMetadata( + public ContainerProtos.ContainerChecksumInfo updateAndGetContainerChecksumFromMetadata( KeyValueContainer container) throws IOException { ContainerMerkleTreeWriter merkleTree = new ContainerMerkleTreeWriter(); try (DBHandle dbHandle = BlockUtils.getDB(container.getContainerData(), conf); @@ -1429,7 +1429,7 @@ private ContainerProtos.ContainerChecksumInfo updateAndGetContainerChecksumFromM private ContainerProtos.ContainerChecksumInfo updateAndGetContainerChecksum(Container container, ContainerMerkleTreeWriter treeWriter, boolean sendICR) throws IOException { - ContainerData containerData = container.getContainerData(); + KeyValueContainerData containerData = (KeyValueContainerData) container.getContainerData(); // Attempt to write the new data checksum to disk. If persisting this fails, keep using the original data // checksum to prevent divergence from what SCM sees in the ICR vs what datanode peers will see when pulling the @@ -1441,6 +1441,14 @@ private ContainerProtos.ContainerChecksumInfo updateAndGetContainerChecksum(Cont if (updatedDataChecksum != originalDataChecksum) { containerData.setDataChecksum(updatedDataChecksum); + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + dbHandle.getStore().getMetadataTable().put(containerData.getContainerDataChecksumKey(), updatedDataChecksum); + } catch (IOException e) { + LOG.error("Failed to update container data checksum in RocksDB for container {}. " + + "Continuing with original checksum for RocksDB {}.", containerData.getContainerID(), + originalDataChecksum, e); + } + String message = "Container data checksum updated from " + checksumToString(originalDataChecksum) + " to " + checksumToString(updatedDataChecksum); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/TarContainerPacker.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/TarContainerPacker.java index 46a2a94975e0..91957ecd7276 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/TarContainerPacker.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/TarContainerPacker.java @@ -24,6 +24,7 @@ import static org.apache.hadoop.hdds.utils.Archiver.readEntry; import static org.apache.hadoop.hdds.utils.Archiver.tar; import static org.apache.hadoop.hdds.utils.Archiver.untar; +import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_DATA_CHECKSUM_EXTENSION; import static org.apache.hadoop.ozone.OzoneConsts.SCHEMA_V3; import java.io.File; @@ -42,6 +43,7 @@ import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State; import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException; import org.apache.hadoop.ozone.OzoneConsts; +import org.apache.hadoop.ozone.container.checksum.ContainerChecksumTreeManager; import org.apache.hadoop.ozone.container.common.interfaces.Container; import org.apache.hadoop.ozone.container.common.interfaces.ContainerPacker; import org.apache.hadoop.ozone.container.keyvalue.helpers.KeyValueContainerLocationUtil; @@ -87,7 +89,10 @@ public byte[] unpackContainerData(Container container, Path dbRoot = getDbPath(containerUntarDir, containerData); Path chunksRoot = getChunkPath(containerUntarDir, containerData); - byte[] descriptorFileContent = innerUnpack(input, dbRoot, chunksRoot); + Path containerMetadataPath = Paths.get(container.getContainerData().getMetadataPath()); + Path tempContainerMetadataPath = Paths.get(containerUntarDir.toString(), + containerMetadataPath.getName(containerMetadataPath.getNameCount() - 1).toString()); + byte[] descriptorFileContent = innerUnpack(input, dbRoot, chunksRoot, tempContainerMetadataPath); if (!Files.exists(destContainerDir)) { Files.createDirectories(destContainerDir); @@ -96,9 +101,6 @@ public byte[] unpackContainerData(Container container, // Before the atomic move, the destination dir is empty and doesn't have a metadata directory. // Writing the .container file will fail as the metadata dir doesn't exist. // So we instead save the container file to the containerUntarDir. - Path containerMetadataPath = Paths.get(container.getContainerData().getMetadataPath()); - Path tempContainerMetadataPath = Paths.get(containerUntarDir.toString(), - containerMetadataPath.getName(containerMetadataPath.getNameCount() - 1).toString()); persistCustomContainerState(container, descriptorFileContent, State.RECOVERING, tempContainerMetadataPath); Files.move(containerUntarDir, destContainerDir, StandardCopyOption.ATOMIC_MOVE, @@ -131,6 +133,11 @@ public void pack(Container container, includeFile(container.getContainerFile(), CONTAINER_FILE_NAME, archiveOutput); + File containerChecksumFile = ContainerChecksumTreeManager.getContainerChecksumFile(containerData); + if (containerChecksumFile.exists()) { + includeFile(containerChecksumFile, containerChecksumFile.getName(), archiveOutput); + } + includePath(getDbPath(containerData), DB_DIR_NAME, archiveOutput); @@ -200,7 +207,7 @@ OutputStream compress(OutputStream output) throws IOException { return compression.wrap(output); } - private byte[] innerUnpack(InputStream input, Path dbRoot, Path chunksRoot) + private byte[] innerUnpack(InputStream input, Path dbRoot, Path chunksRoot, Path tempContainerMetadataPath) throws IOException { byte[] descriptorFileContent = null; try (ArchiveInputStream archiveInput = untar(decompress(input))) { @@ -218,6 +225,10 @@ private byte[] innerUnpack(InputStream input, Path dbRoot, Path chunksRoot) .resolve(name.substring(CHUNKS_DIR_NAME.length() + 1)); extractEntry(entry, archiveInput, size, chunksRoot, destinationPath); + } else if (name.endsWith(CONTAINER_DATA_CHECKSUM_EXTENSION)) { + Path destinationPath = tempContainerMetadataPath.resolve(name); + extractEntry(entry, archiveInput, size, tempContainerMetadataPath, + destinationPath); } else if (CONTAINER_FILE_NAME.equals(name)) { //Don't do anything. Container file should be unpacked in a //separated step by unpackContainerDescriptor call. diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java index fc16e0c71b64..922fb5c6a0e2 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java @@ -278,7 +278,8 @@ public static void parseKVContainerData(KeyValueContainerData kvContainerData, } } - private static void populateContainerDataChecksum(KeyValueContainerData kvContainerData) { + private static void populateContainerDataChecksum(KeyValueContainerData kvContainerData, + Table metadataTable) { if (kvContainerData.isOpen()) { return; } @@ -289,6 +290,8 @@ private static void populateContainerDataChecksum(KeyValueContainerData kvContai if (optionalContainerChecksumInfo.isPresent()) { ContainerChecksumInfo containerChecksumInfo = optionalContainerChecksumInfo.get(); kvContainerData.setDataChecksum(containerChecksumInfo.getContainerMerkleTree().getDataChecksum()); + metadataTable.put(kvContainerData.getContainerDataChecksumKey(), + containerChecksumInfo.getContainerMerkleTree().getDataChecksum()); } } catch (IOException ex) { LOG.warn("Failed to read checksum info for container {}", kvContainerData.getContainerID(), ex); @@ -367,6 +370,16 @@ private static void populateContainerMetadata( kvContainerData.markAsEmpty(); } + // Set container data checksum. + Long containerDataChecksum = metadataTable.get( + kvContainerData.getContainerDataChecksumKey()); + + if (containerDataChecksum != null) { + kvContainerData.setDataChecksum(containerDataChecksum); + } else if (ContainerChecksumTreeManager.hasContainerChecksumFile(kvContainerData)) { + populateContainerDataChecksum(kvContainerData, metadataTable); + } + // Run advanced container inspection/repair operations if specified on // startup. If this method is called but not as a part of startup, // The inspectors will be unloaded and this will be a no-op. @@ -374,7 +387,6 @@ private static void populateContainerMetadata( // Load finalizeBlockLocalIds for container in memory. populateContainerFinalizeBlock(kvContainerData, store); - populateContainerDataChecksum(kvContainerData); } /** diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java index 538fd9c15c6f..762725ac3436 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.container.checksum; +import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_DATA_CHECKSUM_EXTENSION; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.assertContainerDiffMatch; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.assertTreesSortedAndMatch; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.buildTestTree; @@ -97,7 +98,7 @@ public void init() { container = mock(KeyValueContainerData.class); when(container.getContainerID()).thenReturn(CONTAINER_ID); when(container.getMetadataPath()).thenReturn(testDir.getAbsolutePath()); - checksumFile = new File(testDir, CONTAINER_ID + ".tree"); + checksumFile = new File(testDir, CONTAINER_ID + CONTAINER_DATA_CHECKSUM_EXTENSION); checksumManager = new ContainerChecksumTreeManager(new OzoneConfiguration()); metrics = checksumManager.getMetrics(); config = new OzoneConfiguration(); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java index e95fc3ab3ec3..8d465a53f21e 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java @@ -350,13 +350,18 @@ public ContainerProtos.GetContainerChecksumInfoResponseProto getChecksumInfo(lon */ public long checkAndGetDataChecksum(long containerID) { KeyValueContainer container = getContainer(containerID); + KeyValueContainerData containerData = container.getContainerData(); long dataChecksum = 0; try { Optional containerChecksumInfo = - handler.getChecksumManager().read(container.getContainerData()); + handler.getChecksumManager().read(containerData); assertTrue(containerChecksumInfo.isPresent()); dataChecksum = containerChecksumInfo.get().getContainerMerkleTree().getDataChecksum(); - assertEquals(container.getContainerData().getDataChecksum(), dataChecksum); + assertEquals(containerData.getDataChecksum(), dataChecksum); + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + assertEquals(containerData.getDataChecksum(), dbDataChecksum, "DB should have the updated data checksum."); + } } catch (IOException ex) { fail("Failed to read container checksum from disk", ex); } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java index 503d8c0855d8..1f6839c558cd 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java @@ -87,6 +87,7 @@ import org.apache.hadoop.ozone.container.common.impl.ContainerSet; import org.apache.hadoop.ozone.container.common.impl.HddsDispatcher; import org.apache.hadoop.ozone.container.common.interfaces.Container; +import org.apache.hadoop.ozone.container.common.interfaces.DBHandle; import org.apache.hadoop.ozone.container.common.interfaces.Handler; import org.apache.hadoop.ozone.container.common.report.IncrementalReportSender; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; @@ -96,6 +97,7 @@ import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; import org.apache.hadoop.ozone.container.common.volume.StorageVolume; import org.apache.hadoop.ozone.container.common.volume.VolumeSet; +import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils; import org.apache.hadoop.ozone.container.ozoneimpl.ContainerController; import org.apache.hadoop.ozone.container.ozoneimpl.ContainerScannerConfiguration; import org.apache.hadoop.ozone.container.ozoneimpl.OnDemandContainerDataScanner; @@ -679,6 +681,10 @@ public void testUpdateContainerChecksum(ContainerLayoutVersion layoutVersion) th // Initially, container should have no checksum information. assertEquals(0, containerData.getDataChecksum()); + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + assertEquals(0, dbDataChecksum, "DB should have 0 checksum."); + } assertFalse(checksumManager.read(containerData).isPresent()); assertEquals(0, icrCount.get()); @@ -688,6 +694,10 @@ public void testUpdateContainerChecksum(ContainerLayoutVersion layoutVersion) th assertEquals(1, icrCount.get()); // Check checksum in memory. assertEquals(updatedDataChecksum, containerData.getDataChecksum()); + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + assertEquals(updatedDataChecksum, dbDataChecksum, "DB should have the updated data checksum."); + } // Check disk content. ContainerProtos.ContainerChecksumInfo checksumInfo = checksumManager.read(containerData).get(); assertTreesSortedAndMatch(treeWriter.toProto(), checksumInfo.getContainerMerkleTree()); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java index 8228c5182d10..6eeb56e2fc66 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java @@ -20,6 +20,8 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.nio.file.Files.newInputStream; import static java.nio.file.Files.newOutputStream; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.assertTreesSortedAndMatch; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.buildTestTree; import static org.apache.hadoop.ozone.container.keyvalue.TarContainerPacker.CONTAINER_FILE_NAME; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -48,11 +50,14 @@ import org.apache.commons.io.IOUtils; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.utils.Archiver; +import org.apache.hadoop.ozone.container.checksum.ContainerChecksumTreeManager; +import org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeWriter; import org.apache.hadoop.ozone.container.common.impl.ContainerLayoutVersion; import org.apache.hadoop.ozone.container.replication.CopyContainerCompression; import org.apache.ozone.test.SpyInputStream; import org.apache.ozone.test.SpyOutputStream; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -104,6 +109,7 @@ public class TestTarContainerPacker { private ContainerLayoutVersion layout; private String schemaVersion; private OzoneConfiguration conf; + private ContainerChecksumTreeManager checksumTreeManager; private void initTests(ContainerTestVersionInfo versionInfo, CopyContainerCompression compression) { @@ -112,6 +118,7 @@ private void initTests(ContainerTestVersionInfo versionInfo, this.conf = new OzoneConfiguration(); ContainerTestVersionInfo.setTestSchemaVersion(schemaVersion, conf); packer = new TarContainerPacker(compression); + checksumTreeManager = new ContainerChecksumTreeManager(conf); } public static List getLayoutAndCompression() { @@ -140,6 +147,13 @@ public static void cleanup() throws IOException { FileUtils.deleteDirectory(TEMP_DIR.toFile()); } + @AfterEach + public void dirCleanUp() throws IOException { + FileUtils.cleanDirectory(SOURCE_CONTAINER_ROOT.toFile()); + FileUtils.cleanDirectory(DEST_CONTAINER_ROOT.toFile()); + FileUtils.cleanDirectory(TEMP_DIR.toFile()); + } + private static void initDir(Path path) throws IOException { if (path.toFile().exists()) { FileUtils.deleteDirectory(path.toFile()); @@ -148,12 +162,15 @@ private static void initDir(Path path) throws IOException { } private KeyValueContainerData createContainer(Path dir) throws IOException { - return createContainer(dir, true); + return createContainer(dir, true, true); } - private KeyValueContainerData createContainer(Path dir, boolean createDir) + private KeyValueContainerData createContainer(Path dir, boolean createDir, boolean incrementId) throws IOException { - long id = CONTAINER_ID.getAndIncrement(); + long id = CONTAINER_ID.get(); + if (incrementId) { + id = CONTAINER_ID.getAndIncrement(); + } Path containerDir = dir.resolve(String.valueOf(id)); Path dataDir = containerDir.resolve("chunks"); @@ -183,7 +200,7 @@ public void pack(ContainerTestVersionInfo versionInfo, initTests(versionInfo, compression); //GIVEN KeyValueContainerData sourceContainerData = - createContainer(SOURCE_CONTAINER_ROOT); + createContainer(SOURCE_CONTAINER_ROOT, true, false); KeyValueContainer sourceContainer = new KeyValueContainer(sourceContainerData, conf); @@ -194,6 +211,11 @@ public void pack(ContainerTestVersionInfo versionInfo, //sample chunk file in the chunk directory writeChunkFile(sourceContainerData, TEST_CHUNK_FILE_NAME); + //write container checksum file in the metadata directory + ContainerMerkleTreeWriter treeWriter = buildTestTree(conf); + checksumTreeManager.writeContainerDataTree(sourceContainerData, treeWriter); + assertTrue(ContainerChecksumTreeManager.hasContainerChecksumFile(sourceContainerData)); + //sample container descriptor file writeDescriptor(sourceContainer); @@ -239,7 +261,7 @@ public void pack(ContainerTestVersionInfo versionInfo, inputForUnpackDescriptor.assertClosedExactlyOnce(); KeyValueContainerData destinationContainerData = - createContainer(DEST_CONTAINER_ROOT, false); + createContainer(DEST_CONTAINER_ROOT, false, false); KeyValueContainer destinationContainer = new KeyValueContainer(destinationContainerData, conf); @@ -259,6 +281,12 @@ public void pack(ContainerTestVersionInfo versionInfo, assertExampleChunkFileIsGood( Paths.get(destinationContainerData.getChunksPath()), TEST_CHUNK_FILE_NAME); + Files.list(Paths.get(destinationContainerData.getMetadataPath())).forEach(System.out::println); + + assertEquals(sourceContainerData.getContainerID(), destinationContainerData.getContainerID()); + assertTrue(ContainerChecksumTreeManager.hasContainerChecksumFile(destinationContainerData)); + assertTreesSortedAndMatch(checksumTreeManager.read(sourceContainerData).get().getContainerMerkleTree(), + checksumTreeManager.read(destinationContainerData).get().getContainerMerkleTree()); String containerFileData = new String(Files.readAllBytes(destinationContainer.getContainerFile().toPath()), UTF_8); assertTrue(containerFileData.contains("RECOVERING"), @@ -361,7 +389,7 @@ public void unpackContainerDataWithInvalidRelativeChunkFilePath( private KeyValueContainerData unpackContainerData(File containerFile) throws IOException { try (InputStream input = newInputStream(containerFile.toPath())) { - KeyValueContainerData data = createContainer(DEST_CONTAINER_ROOT, false); + KeyValueContainerData data = createContainer(DEST_CONTAINER_ROOT, false, true); KeyValueContainer container = new KeyValueContainer(data, conf); packer.unpackContainerData(container, input, TEMP_DIR, DEST_CONTAINER_ROOT.resolve(String.valueOf(data.getContainerID()))); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java index 243fe218c5e8..62421a4a3223 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java @@ -25,6 +25,7 @@ import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.WRITE_STAGE; import static org.apache.hadoop.ozone.container.common.impl.ContainerImplTestUtils.newContainerSet; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.File; @@ -47,6 +48,7 @@ import org.apache.hadoop.ozone.common.ChunkBuffer; import org.apache.hadoop.ozone.common.ChunkBufferToByteString; import org.apache.hadoop.ozone.container.ContainerTestHelper; +import org.apache.hadoop.ozone.container.checksum.ContainerChecksumTreeManager; import org.apache.hadoop.ozone.container.common.ContainerTestUtils; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.helpers.ChunkInfo; @@ -184,10 +186,11 @@ public void testWriteChunkForClosedContainer() ChunkBuffer writeChunkData = ChunkBuffer.wrap(getData()); KeyValueContainer kvContainer = getKeyValueContainer(); KeyValueContainerData containerData = kvContainer.getContainerData(); - closedKeyValueContainer(); ContainerSet containerSet = newContainerSet(); containerSet.addContainer(kvContainer); KeyValueHandler keyValueHandler = createKeyValueHandler(containerSet); + keyValueHandler.markContainerForClose(kvContainer); + keyValueHandler.closeContainer(kvContainer); keyValueHandler.writeChunkForClosedContainer(getChunkInfo(), getBlockID(), writeChunkData, kvContainer); ChunkBufferToByteString readChunkData = keyValueHandler.getChunkManager().readChunk(kvContainer, getBlockID(), getChunkInfo(), WRITE_STAGE); @@ -230,10 +233,13 @@ public void testWriteChunkForClosedContainer() public void testPutBlockForClosedContainer() throws IOException { KeyValueContainer kvContainer = getKeyValueContainer(); KeyValueContainerData containerData = kvContainer.getContainerData(); - closedKeyValueContainer(); ContainerSet containerSet = newContainerSet(); containerSet.addContainer(kvContainer); KeyValueHandler keyValueHandler = createKeyValueHandler(containerSet); + keyValueHandler.markContainerForClose(kvContainer); + keyValueHandler.closeContainer(kvContainer); + assertEquals(ContainerProtos.ContainerDataProto.State.CLOSED, containerData.getState()); + assertEquals(0L, containerData.getDataChecksum()); List chunkInfoList = new ArrayList<>(); ChunkInfo info = new ChunkInfo(String.format("%d.data.%d", getBlockID().getLocalID(), 0), 0L, 20L); @@ -244,9 +250,12 @@ public void testPutBlockForClosedContainer() throws IOException { ChunkBuffer chunkData = ContainerTestHelper.getData(20); keyValueHandler.writeChunkForClosedContainer(info, getBlockID(), chunkData, kvContainer); keyValueHandler.putBlockForClosedContainer(kvContainer, putBlockData, 1L, true); + ContainerProtos.ContainerChecksumInfo containerChecksumInfo = + keyValueHandler.updateAndGetContainerChecksumFromMetadata(kvContainer); assertEquals(1L, containerData.getBlockCommitSequenceId()); assertEquals(1L, containerData.getBlockCount()); assertEquals(20L, containerData.getBytesUsed()); + assertEquals(ContainerChecksumTreeManager.getDataChecksum(containerChecksumInfo), containerData.getDataChecksum()); try (DBHandle dbHandle = BlockUtils.getDB(containerData, new OzoneConfiguration())) { long localID = putBlockData.getLocalID(); @@ -254,6 +263,8 @@ public void testPutBlockForClosedContainer() throws IOException { .get(containerData.getBlockKey(localID)); Assertions.assertTrue(blockDataEquals(putBlockData, getBlockData)); assertEquals(20L, dbHandle.getStore().getMetadataTable().get(containerData.getBytesUsedKey())); + assertEquals(containerData.getDataChecksum(), dbHandle.getStore().getMetadataTable() + .get(containerData.getContainerDataChecksumKey())); } // Add another chunk and check the put block data @@ -264,6 +275,10 @@ public void testPutBlockForClosedContainer() throws IOException { chunkData = ContainerTestHelper.getData(20); keyValueHandler.writeChunkForClosedContainer(newChunkInfo, getBlockID(), chunkData, kvContainer); keyValueHandler.putBlockForClosedContainer(kvContainer, putBlockData, 2L, true); + long previousDataChecksum = containerData.getDataChecksum(); + containerChecksumInfo = keyValueHandler.updateAndGetContainerChecksumFromMetadata(kvContainer); + assertNotEquals(previousDataChecksum, containerData.getDataChecksum()); + assertEquals(ContainerChecksumTreeManager.getDataChecksum(containerChecksumInfo), containerData.getDataChecksum()); assertEquals(2L, containerData.getBlockCommitSequenceId()); assertEquals(1L, containerData.getBlockCount()); assertEquals(40L, containerData.getBytesUsed()); @@ -274,6 +289,8 @@ public void testPutBlockForClosedContainer() throws IOException { .get(containerData.getBlockKey(localID)); Assertions.assertTrue(blockDataEquals(putBlockData, getBlockData)); assertEquals(40L, dbHandle.getStore().getMetadataTable().get(containerData.getBytesUsedKey())); + assertEquals(containerData.getDataChecksum(), dbHandle.getStore().getMetadataTable() + .get(containerData.getContainerDataChecksumKey())); } // Replace the last chunk with a chunk of greater size, This should only update the bytesUsed with @@ -331,10 +348,6 @@ public KeyValueHandler createKeyValueHandler(ContainerSet containerSet) return ContainerTestUtils.getKeyValueHandler(conf, dnUuid, containerSet, volumeSet); } - public void closedKeyValueContainer() { - getKeyValueContainer().getContainerData().setState(ContainerProtos.ContainerDataProto.State.CLOSED); - } - @Override protected ContainerLayoutTestInfo getStrategy() { return ContainerLayoutTestInfo.FILE_PER_BLOCK; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java index 2306af221c43..cf16711f32f9 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java @@ -30,8 +30,11 @@ import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State; import org.apache.hadoop.ozone.container.common.interfaces.Container; +import org.apache.hadoop.ozone.container.common.interfaces.DBHandle; import org.apache.hadoop.ozone.container.common.utils.ContainerLogger; +import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; import org.apache.hadoop.ozone.container.keyvalue.TestContainerCorruptions; +import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils; import org.apache.hadoop.ozone.container.ozoneimpl.BackgroundContainerDataScanner; import org.apache.hadoop.ozone.container.ozoneimpl.ContainerScannerConfiguration; import org.apache.ozone.test.GenericTestUtils; @@ -78,6 +81,7 @@ void testCorruptionDetected(TestContainerCorruptions corruption) throws Exception { pauseScanner(); + OzoneConfiguration conf = new OzoneConfiguration(); long containerID = writeDataThenCloseContainer(); // Container corruption has not yet been introduced. Container container = getDnContainer(containerID); @@ -111,6 +115,11 @@ void testCorruptionDetected(TestContainerCorruptions corruption) assertTrue(containerChecksumFileExists(containerID)); ContainerProtos.ContainerChecksumInfo updatedChecksumInfo = readChecksumFile(container.getContainerData()); assertEquals(newReportedDataChecksum, updatedChecksumInfo.getContainerMerkleTree().getDataChecksum()); + KeyValueContainerData containerData = (KeyValueContainerData) container.getContainerData(); + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + assertEquals(newReportedDataChecksum, dbDataChecksum, "DB should have the updated data checksum."); + } } if (corruption == TestContainerCorruptions.TRUNCATED_BLOCK || diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java index d3b3ed46fdeb..f3a9908865bd 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java @@ -30,8 +30,11 @@ import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State; import org.apache.hadoop.ozone.container.common.interfaces.Container; +import org.apache.hadoop.ozone.container.common.interfaces.DBHandle; import org.apache.hadoop.ozone.container.common.utils.ContainerLogger; +import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; import org.apache.hadoop.ozone.container.keyvalue.TestContainerCorruptions; +import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils; import org.apache.hadoop.ozone.container.ozoneimpl.ContainerScannerConfiguration; import org.apache.hadoop.ozone.container.ozoneimpl.OnDemandContainerDataScanner; import org.apache.ozone.test.GenericTestUtils; @@ -98,6 +101,7 @@ static void init() throws Exception { void testCorruptionDetected(TestContainerCorruptions corruption) throws Exception { String keyName = "testKey"; + OzoneConfiguration conf = new OzoneConfiguration(); long containerID = writeDataThenCloseContainer(keyName); // Container corruption has not yet been introduced. Container container = getDnContainer(containerID); @@ -135,6 +139,11 @@ void testCorruptionDetected(TestContainerCorruptions corruption) assertTrue(containerChecksumFileExists(containerID)); ContainerProtos.ContainerChecksumInfo updatedChecksumInfo = readChecksumFile(container.getContainerData()); assertEquals(newReportedDataChecksum, updatedChecksumInfo.getContainerMerkleTree().getDataChecksum()); + KeyValueContainerData containerData = (KeyValueContainerData) container.getContainerData(); + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + assertEquals(newReportedDataChecksum, dbDataChecksum, "DB should have the updated data checksum."); + } } } } From 93d396ab2456f39dcfcc044d8a81e7a69368d087 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Thu, 12 Jun 2025 15:57:39 +0530 Subject: [PATCH 02/10] Address review comments. --- .../ContainerChecksumTreeManager.java | 1 - .../container/keyvalue/KeyValueHandler.java | 6 +++- .../helpers/KeyValueContainerUtil.java | 26 ++++++++--------- .../ContainerMerkleTreeTestUtils.java | 28 +++++++++++++++++++ .../TestContainerChecksumTreeManager.java | 5 ++-- ...tainerReconciliationWithMockDatanodes.java | 7 ++--- .../keyvalue/TestKeyValueHandler.java | 17 ++++------- .../impl/TestFilePerBlockStrategy.java | 24 +++++++--------- ...groundContainerDataScannerIntegration.java | 8 ++---- ...DemandContainerDataScannerIntegration.java | 8 ++---- 10 files changed, 69 insertions(+), 61 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 0c294000f6b2..356226a1bde7 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 @@ -315,7 +315,6 @@ public static boolean hasContainerChecksumFile(ContainerData data) { /** * Returns the container checksum tree file for the specified container without deserializing it. */ - @VisibleForTesting public static File getContainerChecksumFile(ContainerData data) { return new File(data.getMetadataPath(), data.getContainerID() + CONTAINER_DATA_CHECKSUM_EXTENSION); } 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 972c6c8dab24..3f616d7cebec 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 @@ -1409,6 +1409,7 @@ private void updateContainerChecksumFromMetadataIfNeeded(Container container) { * This method does not send an ICR with the updated checksum info. * @param container - Container for which the container merkle tree needs to be updated. */ + @VisibleForTesting public ContainerProtos.ContainerChecksumInfo updateAndGetContainerChecksumFromMetadata( KeyValueContainer container) throws IOException { ContainerMerkleTreeWriter merkleTree = new ContainerMerkleTreeWriter(); @@ -1442,11 +1443,14 @@ private ContainerProtos.ContainerChecksumInfo updateAndGetContainerChecksum(Cont if (updatedDataChecksum != originalDataChecksum) { containerData.setDataChecksum(updatedDataChecksum); try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + // This value is only used during the datanode startup. If the update fails, then it's okay as the merkle tree + // and in-memory checksum will still be the same. This will be updated when the next time we update the tree. + // Either scanner or reconciliation will update the checksum. dbHandle.getStore().getMetadataTable().put(containerData.getContainerDataChecksumKey(), updatedDataChecksum); } catch (IOException e) { LOG.error("Failed to update container data checksum in RocksDB for container {}. " + "Continuing with original checksum for RocksDB {}.", containerData.getContainerID(), - originalDataChecksum, e); + checksumToString(originalDataChecksum), e); } String message = diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java index 922fb5c6a0e2..ce27055f3d63 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java @@ -278,20 +278,26 @@ public static void parseKVContainerData(KeyValueContainerData kvContainerData, } } - private static void populateContainerDataChecksum(KeyValueContainerData kvContainerData, - Table metadataTable) { + private static void loadAndSetContainerDataChecksum(KeyValueContainerData kvContainerData, + Table metadataTable) throws IOException { if (kvContainerData.isOpen()) { return; } + Long containerDataChecksum = metadataTable.get(kvContainerData.getContainerDataChecksumKey()); + if (containerDataChecksum != null) { + kvContainerData.setDataChecksum(containerDataChecksum); + return; + } + try { Optional optionalContainerChecksumInfo = ContainerChecksumTreeManager .readChecksumInfo(kvContainerData); if (optionalContainerChecksumInfo.isPresent()) { ContainerChecksumInfo containerChecksumInfo = optionalContainerChecksumInfo.get(); - kvContainerData.setDataChecksum(containerChecksumInfo.getContainerMerkleTree().getDataChecksum()); - metadataTable.put(kvContainerData.getContainerDataChecksumKey(), - containerChecksumInfo.getContainerMerkleTree().getDataChecksum()); + containerDataChecksum = containerChecksumInfo.getContainerMerkleTree().getDataChecksum(); + kvContainerData.setDataChecksum(containerDataChecksum); + metadataTable.put(kvContainerData.getContainerDataChecksumKey(), containerDataChecksum); } } catch (IOException ex) { LOG.warn("Failed to read checksum info for container {}", kvContainerData.getContainerID(), ex); @@ -370,15 +376,7 @@ private static void populateContainerMetadata( kvContainerData.markAsEmpty(); } - // Set container data checksum. - Long containerDataChecksum = metadataTable.get( - kvContainerData.getContainerDataChecksumKey()); - - if (containerDataChecksum != null) { - kvContainerData.setDataChecksum(containerDataChecksum); - } else if (ContainerChecksumTreeManager.hasContainerChecksumFile(kvContainerData)) { - populateContainerDataChecksum(kvContainerData, metadataTable); - } + loadAndSetContainerDataChecksum(kvContainerData, metadataTable); // Run advanced container inspection/repair operations if specified on // startup. If this method is called but not as a part of startup, diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java index 981b0960f670..a29d4bdd512d 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java @@ -34,11 +34,13 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Random; import java.util.Set; import java.util.stream.Collectors; import org.apache.commons.lang3.tuple.Pair; import org.apache.hadoop.hdds.conf.ConfigurationSource; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.conf.StorageUnit; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.hdds.scm.OzoneClientConfig; @@ -46,6 +48,9 @@ import org.apache.hadoop.ozone.HddsDatanodeService; import org.apache.hadoop.ozone.container.common.impl.ContainerData; import org.apache.hadoop.ozone.container.common.interfaces.Container; +import org.apache.hadoop.ozone.container.common.interfaces.DBHandle; +import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; +import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils; import org.apache.hadoop.ozone.container.ozoneimpl.OzoneContainer; import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; @@ -353,4 +358,27 @@ public static void writeContainerDataTreeProto(ContainerData data, ContainerProt } data.setDataChecksum(checksumInfo.getContainerMerkleTree().getDataChecksum()); } + + /** + * This function verifies that the in-memory data checksum matches the one stored in the container data and + * the RocksDB. + * + * @param containerData The container data to verify. + * @param conf The Ozone configuration. + * @throws IOException If an error occurs while reading the checksum info or RocksDB. + */ + public static void verifyAllDataChecksumMatches(KeyValueContainerData containerData, OzoneConfiguration conf) + throws IOException { + assertNotNull(containerData, "Container data should not be null"); + Optional containerChecksumInfo = ContainerChecksumTreeManager + .readChecksumInfo(containerData); + assertTrue(containerChecksumInfo.isPresent()); + long dataChecksum = containerChecksumInfo.get().getContainerMerkleTree().getDataChecksum(); + assertEquals(containerData.getDataChecksum(), dataChecksum, "In-memory data checksum should match " + + "the one in the checksum file."); + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + assertEquals(containerData.getDataChecksum(), dbDataChecksum, "DB should have the updated data checksum."); + } + } } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java index 762725ac3436..d5d8f7a08f0a 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java @@ -17,7 +17,6 @@ package org.apache.hadoop.ozone.container.checksum; -import static org.apache.hadoop.ozone.OzoneConsts.CONTAINER_DATA_CHECKSUM_EXTENSION; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.assertContainerDiffMatch; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.assertTreesSortedAndMatch; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.buildTestTree; @@ -98,7 +97,9 @@ public void init() { container = mock(KeyValueContainerData.class); when(container.getContainerID()).thenReturn(CONTAINER_ID); when(container.getMetadataPath()).thenReturn(testDir.getAbsolutePath()); - checksumFile = new File(testDir, CONTAINER_ID + CONTAINER_DATA_CHECKSUM_EXTENSION); + // .tree is hardcoded here to check if the checksum file extension has changed. + // As a change in the extension will result in incompatibility. + checksumFile = new File(testDir, CONTAINER_ID + ".tree"); checksumManager = new ContainerChecksumTreeManager(new OzoneConfiguration()); metrics = checksumManager.getMetrics(); config = new OzoneConfiguration(); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java index 8d465a53f21e..56c97c566e93 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java @@ -20,6 +20,7 @@ import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS; import static org.apache.hadoop.hdds.protocol.MockDatanodeDetails.randomDatanodeDetails; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_KEY; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumMatches; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.WRITE_STAGE; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.createDbInstancesForTestIfNeeded; import static org.apache.hadoop.ozone.container.common.impl.ContainerImplTestUtils.newContainerSet; @@ -357,11 +358,7 @@ public long checkAndGetDataChecksum(long containerID) { handler.getChecksumManager().read(containerData); assertTrue(containerChecksumInfo.isPresent()); dataChecksum = containerChecksumInfo.get().getContainerMerkleTree().getDataChecksum(); - assertEquals(containerData.getDataChecksum(), dataChecksum); - try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { - Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); - assertEquals(containerData.getDataChecksum(), dbDataChecksum, "DB should have the updated data checksum."); - } + verifyAllDataChecksumMatches(containerData, conf); } catch (IOException ex) { fail("Failed to read container checksum from disk", ex); } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java index 1f6839c558cd..c930320f62f9 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java @@ -27,6 +27,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.GB; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.assertTreesSortedAndMatch; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.buildTestTree; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumMatches; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.createBlockMetaData; import static org.apache.hadoop.ozone.container.common.impl.ContainerImplTestUtils.newContainerSet; import static org.assertj.core.api.Assertions.assertThat; @@ -87,7 +88,6 @@ import org.apache.hadoop.ozone.container.common.impl.ContainerSet; import org.apache.hadoop.ozone.container.common.impl.HddsDispatcher; import org.apache.hadoop.ozone.container.common.interfaces.Container; -import org.apache.hadoop.ozone.container.common.interfaces.DBHandle; import org.apache.hadoop.ozone.container.common.interfaces.Handler; import org.apache.hadoop.ozone.container.common.report.IncrementalReportSender; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; @@ -97,7 +97,6 @@ import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; import org.apache.hadoop.ozone.container.common.volume.StorageVolume; import org.apache.hadoop.ozone.container.common.volume.VolumeSet; -import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils; import org.apache.hadoop.ozone.container.ozoneimpl.ContainerController; import org.apache.hadoop.ozone.container.ozoneimpl.ContainerScannerConfiguration; import org.apache.hadoop.ozone.container.ozoneimpl.OnDemandContainerDataScanner; @@ -681,10 +680,8 @@ public void testUpdateContainerChecksum(ContainerLayoutVersion layoutVersion) th // Initially, container should have no checksum information. assertEquals(0, containerData.getDataChecksum()); - try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { - Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); - assertEquals(0, dbDataChecksum, "DB should have 0 checksum."); - } + // Check all data checksums are updated correctly. + verifyAllDataChecksumMatches(containerData, conf); assertFalse(checksumManager.read(containerData).isPresent()); assertEquals(0, icrCount.get()); @@ -692,12 +689,8 @@ public void testUpdateContainerChecksum(ContainerLayoutVersion layoutVersion) th keyValueHandler.updateContainerChecksum(container, treeWriter); // Check ICR sent. The ICR sender verifies that the expected checksum is present in the report. assertEquals(1, icrCount.get()); - // Check checksum in memory. - assertEquals(updatedDataChecksum, containerData.getDataChecksum()); - try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { - Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); - assertEquals(updatedDataChecksum, dbDataChecksum, "DB should have the updated data checksum."); - } + // Check all data checksums are updated correctly. + verifyAllDataChecksumMatches(containerData, conf); // Check disk content. ContainerProtos.ContainerChecksumInfo checksumInfo = checksumManager.read(containerData).get(); assertTreesSortedAndMatch(treeWriter.toProto(), checksumInfo.getContainerMerkleTree()); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java index 62421a4a3223..cfcaa2197d31 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java @@ -22,6 +22,7 @@ import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_KEY; import static org.apache.hadoop.ozone.container.ContainerTestHelper.getChunk; import static org.apache.hadoop.ozone.container.ContainerTestHelper.setDataChecksum; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumMatches; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.WRITE_STAGE; import static org.apache.hadoop.ozone.container.common.impl.ContainerImplTestUtils.newContainerSet; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -48,7 +49,6 @@ import org.apache.hadoop.ozone.common.ChunkBuffer; import org.apache.hadoop.ozone.common.ChunkBufferToByteString; import org.apache.hadoop.ozone.container.ContainerTestHelper; -import org.apache.hadoop.ozone.container.checksum.ContainerChecksumTreeManager; import org.apache.hadoop.ozone.container.common.ContainerTestUtils; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.helpers.ChunkInfo; @@ -231,6 +231,7 @@ public void testWriteChunkForClosedContainer() @Test public void testPutBlockForClosedContainer() throws IOException { + OzoneConfiguration conf = new OzoneConfiguration(); KeyValueContainer kvContainer = getKeyValueContainer(); KeyValueContainerData containerData = kvContainer.getContainerData(); ContainerSet containerSet = newContainerSet(); @@ -250,21 +251,18 @@ public void testPutBlockForClosedContainer() throws IOException { ChunkBuffer chunkData = ContainerTestHelper.getData(20); keyValueHandler.writeChunkForClosedContainer(info, getBlockID(), chunkData, kvContainer); keyValueHandler.putBlockForClosedContainer(kvContainer, putBlockData, 1L, true); - ContainerProtos.ContainerChecksumInfo containerChecksumInfo = - keyValueHandler.updateAndGetContainerChecksumFromMetadata(kvContainer); + keyValueHandler.updateAndGetContainerChecksumFromMetadata(kvContainer); assertEquals(1L, containerData.getBlockCommitSequenceId()); assertEquals(1L, containerData.getBlockCount()); assertEquals(20L, containerData.getBytesUsed()); - assertEquals(ContainerChecksumTreeManager.getDataChecksum(containerChecksumInfo), containerData.getDataChecksum()); + verifyAllDataChecksumMatches(containerData, conf); - try (DBHandle dbHandle = BlockUtils.getDB(containerData, new OzoneConfiguration())) { + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { long localID = putBlockData.getLocalID(); BlockData getBlockData = dbHandle.getStore().getBlockDataTable() .get(containerData.getBlockKey(localID)); Assertions.assertTrue(blockDataEquals(putBlockData, getBlockData)); assertEquals(20L, dbHandle.getStore().getMetadataTable().get(containerData.getBytesUsedKey())); - assertEquals(containerData.getDataChecksum(), dbHandle.getStore().getMetadataTable() - .get(containerData.getContainerDataChecksumKey())); } // Add another chunk and check the put block data @@ -276,21 +274,19 @@ public void testPutBlockForClosedContainer() throws IOException { keyValueHandler.writeChunkForClosedContainer(newChunkInfo, getBlockID(), chunkData, kvContainer); keyValueHandler.putBlockForClosedContainer(kvContainer, putBlockData, 2L, true); long previousDataChecksum = containerData.getDataChecksum(); - containerChecksumInfo = keyValueHandler.updateAndGetContainerChecksumFromMetadata(kvContainer); - assertNotEquals(previousDataChecksum, containerData.getDataChecksum()); - assertEquals(ContainerChecksumTreeManager.getDataChecksum(containerChecksumInfo), containerData.getDataChecksum()); + keyValueHandler.updateAndGetContainerChecksumFromMetadata(kvContainer); assertEquals(2L, containerData.getBlockCommitSequenceId()); assertEquals(1L, containerData.getBlockCount()); assertEquals(40L, containerData.getBytesUsed()); + assertNotEquals(previousDataChecksum, containerData.getDataChecksum()); + verifyAllDataChecksumMatches(containerData, conf); - try (DBHandle dbHandle = BlockUtils.getDB(containerData, new OzoneConfiguration())) { + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { long localID = putBlockData.getLocalID(); BlockData getBlockData = dbHandle.getStore().getBlockDataTable() .get(containerData.getBlockKey(localID)); Assertions.assertTrue(blockDataEquals(putBlockData, getBlockData)); assertEquals(40L, dbHandle.getStore().getMetadataTable().get(containerData.getBytesUsedKey())); - assertEquals(containerData.getDataChecksum(), dbHandle.getStore().getMetadataTable() - .get(containerData.getContainerDataChecksumKey())); } // Replace the last chunk with a chunk of greater size, This should only update the bytesUsed with @@ -308,7 +304,7 @@ public void testPutBlockForClosedContainer() throws IOException { // Old chunk size 20, new chunk size 30, difference 10. So bytesUsed should be 40 + 10 = 50 assertEquals(50L, containerData.getBytesUsed()); - try (DBHandle dbHandle = BlockUtils.getDB(containerData, new OzoneConfiguration())) { + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { long localID = putBlockData.getLocalID(); BlockData getBlockData = dbHandle.getStore().getBlockDataTable() .get(containerData.getBlockKey(localID)); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java index cf16711f32f9..1b9b658556c2 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java @@ -20,6 +20,7 @@ import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.CLOSED; import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.UNHEALTHY; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.readChecksumFile; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumMatches; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -30,11 +31,9 @@ import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State; import org.apache.hadoop.ozone.container.common.interfaces.Container; -import org.apache.hadoop.ozone.container.common.interfaces.DBHandle; import org.apache.hadoop.ozone.container.common.utils.ContainerLogger; import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; import org.apache.hadoop.ozone.container.keyvalue.TestContainerCorruptions; -import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils; import org.apache.hadoop.ozone.container.ozoneimpl.BackgroundContainerDataScanner; import org.apache.hadoop.ozone.container.ozoneimpl.ContainerScannerConfiguration; import org.apache.ozone.test.GenericTestUtils; @@ -116,10 +115,7 @@ void testCorruptionDetected(TestContainerCorruptions corruption) ContainerProtos.ContainerChecksumInfo updatedChecksumInfo = readChecksumFile(container.getContainerData()); assertEquals(newReportedDataChecksum, updatedChecksumInfo.getContainerMerkleTree().getDataChecksum()); KeyValueContainerData containerData = (KeyValueContainerData) container.getContainerData(); - try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { - Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); - assertEquals(newReportedDataChecksum, dbDataChecksum, "DB should have the updated data checksum."); - } + verifyAllDataChecksumMatches(containerData, conf); } if (corruption == TestContainerCorruptions.TRUNCATED_BLOCK || diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java index f3a9908865bd..156c876bed59 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java @@ -20,6 +20,7 @@ import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.CLOSED; import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.UNHEALTHY; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.readChecksumFile; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumMatches; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -30,11 +31,9 @@ import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State; import org.apache.hadoop.ozone.container.common.interfaces.Container; -import org.apache.hadoop.ozone.container.common.interfaces.DBHandle; import org.apache.hadoop.ozone.container.common.utils.ContainerLogger; import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; import org.apache.hadoop.ozone.container.keyvalue.TestContainerCorruptions; -import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils; import org.apache.hadoop.ozone.container.ozoneimpl.ContainerScannerConfiguration; import org.apache.hadoop.ozone.container.ozoneimpl.OnDemandContainerDataScanner; import org.apache.ozone.test.GenericTestUtils; @@ -140,10 +139,7 @@ void testCorruptionDetected(TestContainerCorruptions corruption) ContainerProtos.ContainerChecksumInfo updatedChecksumInfo = readChecksumFile(container.getContainerData()); assertEquals(newReportedDataChecksum, updatedChecksumInfo.getContainerMerkleTree().getDataChecksum()); KeyValueContainerData containerData = (KeyValueContainerData) container.getContainerData(); - try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { - Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); - assertEquals(newReportedDataChecksum, dbDataChecksum, "DB should have the updated data checksum."); - } + verifyAllDataChecksumMatches(containerData, conf); } } } From 29501208dcfca7eb7e63ae7a7ebacbf552c7ee11 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Mon, 7 Jul 2025 17:04:06 -0700 Subject: [PATCH 03/10] Address review comments. --- .../ContainerChecksumTreeManager.java | 10 +-- .../container/keyvalue/KeyValueHandler.java | 4 +- .../keyvalue/TarContainerPacker.java | 26 ++++--- .../helpers/KeyValueContainerUtil.java | 7 +- .../ContainerMerkleTreeTestUtils.java | 9 ++- .../TestContainerChecksumTreeManager.java | 4 +- ...tainerReconciliationWithMockDatanodes.java | 4 +- .../keyvalue/TestKeyValueHandler.java | 6 +- .../keyvalue/TestTarContainerPacker.java | 70 +++++-------------- .../impl/TestFilePerBlockStrategy.java | 6 +- ...groundContainerDataScannerIntegration.java | 5 +- ...stContainerScannerIntegrationAbstract.java | 4 ++ ...DemandContainerDataScannerIntegration.java | 5 +- 13 files changed, 68 insertions(+), 92 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 868ac9157ece..1eeee187d824 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 @@ -24,6 +24,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.Striped; +import jakarta.annotation.Nullable; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -350,7 +351,8 @@ private Lock getLock(long containerID) { */ public Optional read(ContainerData data) throws IOException { try { - return captureLatencyNs(metrics.getReadContainerMerkleTreeLatencyNS(), () -> readChecksumInfo(data)); + return captureLatencyNs(metrics.getReadContainerMerkleTreeLatencyNS(), () -> + Optional.ofNullable(readChecksumInfo(data))); } catch (IOException ex) { metrics.incrementMerkleTreeReadFailures(); throw new IOException(ex); @@ -418,17 +420,17 @@ public ByteString getContainerChecksumInfo(KeyValueContainerData data) throws IO * Callers are not required to hold a lock while calling this since writes are done to a tmp file and atomically * swapped into place. */ - public static Optional readChecksumInfo(ContainerData data) + public static @Nullable ContainerProtos.ContainerChecksumInfo readChecksumInfo(ContainerData data) throws IOException { long containerID = data.getContainerID(); File checksumFile = getContainerChecksumFile(data); try { if (!checksumFile.exists()) { LOG.debug("No checksum file currently exists for container {} at the path {}", containerID, checksumFile); - return Optional.empty(); + return null; } try (InputStream inStream = Files.newInputStream(checksumFile.toPath())) { - return Optional.of(ContainerProtos.ContainerChecksumInfo.parseFrom(inStream)); + return ContainerProtos.ContainerChecksumInfo.parseFrom(inStream); } } catch (IOException ex) { throw new IOException("Error occurred when reading container merkle tree for containerID " 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 210a2a20bf26..cc6e5a6b7b9f 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 @@ -1444,12 +1444,12 @@ private ContainerProtos.ContainerChecksumInfo updateAndGetContainerChecksum(Cont containerData.setDataChecksum(updatedDataChecksum); try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { // This value is only used during the datanode startup. If the update fails, then it's okay as the merkle tree - // and in-memory checksum will still be the same. This will be updated when the next time we update the tree. + // and in-memory checksum will still be the same. This will be updated the next time we update the tree. // Either scanner or reconciliation will update the checksum. dbHandle.getStore().getMetadataTable().put(containerData.getContainerDataChecksumKey(), updatedDataChecksum); } catch (IOException e) { LOG.error("Failed to update container data checksum in RocksDB for container {}. " + - "Continuing with original checksum for RocksDB {}.", containerData.getContainerID(), + "Leaving the original checksum in RocksDB: {}", containerData.getContainerID(), checksumToString(originalDataChecksum), e); } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/TarContainerPacker.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/TarContainerPacker.java index ec4c4e9b8459..9ab8a8c58063 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/TarContainerPacker.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/TarContainerPacker.java @@ -47,6 +47,7 @@ import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.container.checksum.ContainerChecksumTreeManager; import org.apache.hadoop.ozone.container.common.helpers.ContainerUtils; +import org.apache.hadoop.ozone.container.common.impl.ContainerData; import org.apache.hadoop.ozone.container.common.impl.ContainerDataYaml; import org.apache.hadoop.ozone.container.common.interfaces.Container; import org.apache.hadoop.ozone.container.common.interfaces.ContainerPacker; @@ -94,10 +95,8 @@ public byte[] unpackContainerData(Container container, } Path dbRoot = getDbPath(containerUntarDir, containerData); - Path chunksRoot = getChunkPath(containerUntarDir, containerData); - Path containerMetadataPath = Paths.get(container.getContainerData().getMetadataPath()); - Path tempContainerMetadataPath = Paths.get(containerUntarDir.toString(), - containerMetadataPath.getName(containerMetadataPath.getNameCount() - 1).toString()); + Path chunksRoot = getChunkPath(containerUntarDir); + Path tempContainerMetadataPath = getTempContainerMetadataPath(containerUntarDir, containerData); byte[] descriptorFileContent = innerUnpack(input, dbRoot, chunksRoot, tempContainerMetadataPath); if (!Files.exists(destContainerDir)) { @@ -209,11 +208,20 @@ public static Path getDbPath(Path baseDir, } } - public static Path getChunkPath(Path baseDir, - KeyValueContainerData containerData) { + public static Path getChunkPath(Path baseDir) { return KeyValueContainerLocationUtil.getChunksLocationPath(baseDir.toString()).toPath(); } + private Path getContainerMetadataPath(ContainerData containerData) { + return Paths.get(containerData.getMetadataPath()); + } + + private Path getTempContainerMetadataPath(Path containerUntarDir, ContainerData containerData) { + Path containerMetadataPath = getContainerMetadataPath(containerData); + return Paths.get(containerUntarDir.toString(), + containerMetadataPath.getName(containerMetadataPath.getNameCount() - 1).toString()); + } + InputStream decompress(InputStream input) throws IOException { return compression.wrap(input); } @@ -222,7 +230,7 @@ OutputStream compress(OutputStream output) throws IOException { return compression.wrap(output); } - private byte[] innerUnpack(InputStream input, Path dbRoot, Path chunksRoot, Path tempContainerMetadataPath) + private byte[] innerUnpack(InputStream input, Path dbRoot, Path chunksRoot, Path metadataRoot) throws IOException { byte[] descriptorFileContent = null; try (ArchiveInputStream archiveInput = untar(decompress(input))) { @@ -241,8 +249,8 @@ private byte[] innerUnpack(InputStream input, Path dbRoot, Path chunksRoot, Path extractEntry(entry, archiveInput, size, chunksRoot, destinationPath); } else if (name.endsWith(CONTAINER_DATA_CHECKSUM_EXTENSION)) { - Path destinationPath = tempContainerMetadataPath.resolve(name); - extractEntry(entry, archiveInput, size, tempContainerMetadataPath, + Path destinationPath = metadataRoot.resolve(name); + extractEntry(entry, archiveInput, size, metadataRoot, destinationPath); } else if (CONTAINER_FILE_NAME.equals(name)) { //Don't do anything. Container file should be unpacked in a diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java index 06b7f5bf981f..08182e4fb861 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java @@ -26,7 +26,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Optional; import org.apache.commons.io.FileUtils; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; @@ -308,10 +307,8 @@ private static void loadAndSetContainerDataChecksum(KeyValueContainerData kvCont } try { - Optional optionalContainerChecksumInfo = ContainerChecksumTreeManager - .readChecksumInfo(kvContainerData); - if (optionalContainerChecksumInfo.isPresent()) { - ContainerChecksumInfo containerChecksumInfo = optionalContainerChecksumInfo.get(); + ContainerChecksumInfo containerChecksumInfo = ContainerChecksumTreeManager.readChecksumInfo(kvContainerData); + if (containerChecksumInfo != null) { containerDataChecksum = containerChecksumInfo.getContainerMerkleTree().getDataChecksum(); kvContainerData.setDataChecksum(containerDataChecksum); metadataTable.put(kvContainerData.getContainerDataChecksumKey(), containerDataChecksum); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java index 5d069dc24d59..6a01f7dc58d8 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java @@ -34,7 +34,6 @@ import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.Random; import java.util.Set; import java.util.stream.Collectors; @@ -367,13 +366,13 @@ public static void writeContainerDataTreeProto(ContainerData data, ContainerProt * @param conf The Ozone configuration. * @throws IOException If an error occurs while reading the checksum info or RocksDB. */ - public static void verifyAllDataChecksumMatches(KeyValueContainerData containerData, OzoneConfiguration conf) + public static void verifyAllDataChecksumsMatch(KeyValueContainerData containerData, OzoneConfiguration conf) throws IOException { assertNotNull(containerData, "Container data should not be null"); - Optional containerChecksumInfo = ContainerChecksumTreeManager + ContainerProtos.ContainerChecksumInfo containerChecksumInfo = ContainerChecksumTreeManager .readChecksumInfo(containerData); - assertTrue(containerChecksumInfo.isPresent()); - long dataChecksum = containerChecksumInfo.get().getContainerMerkleTree().getDataChecksum(); + assertNotNull(containerChecksumInfo); + long dataChecksum = containerChecksumInfo.getContainerMerkleTree().getDataChecksum(); assertEquals(containerData.getDataChecksum(), dataChecksum, "In-memory data checksum should match " + "the one in the checksum file."); try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java index 6cb44555c63d..9e107bc647d5 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/TestContainerChecksumTreeManager.java @@ -98,8 +98,8 @@ public void init() { container = mock(KeyValueContainerData.class); when(container.getContainerID()).thenReturn(CONTAINER_ID); when(container.getMetadataPath()).thenReturn(testDir.getAbsolutePath()); - // .tree is hardcoded here to check if the checksum file extension has changed. - // As a change in the extension will result in incompatibility. + // File name is hardcoded here to check if the file name has been changed, since this would + // need additional compatibility handling. checksumFile = new File(testDir, CONTAINER_ID + ".tree"); checksumManager = new ContainerChecksumTreeManager(new OzoneConfiguration()); metrics = checksumManager.getMetrics(); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java index 56c97c566e93..40c9a67b30ac 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java @@ -20,7 +20,7 @@ import static org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS; import static org.apache.hadoop.hdds.protocol.MockDatanodeDetails.randomDatanodeDetails; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_KEY; -import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumMatches; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumsMatch; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.WRITE_STAGE; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.createDbInstancesForTestIfNeeded; import static org.apache.hadoop.ozone.container.common.impl.ContainerImplTestUtils.newContainerSet; @@ -358,7 +358,7 @@ public long checkAndGetDataChecksum(long containerID) { handler.getChecksumManager().read(containerData); assertTrue(containerChecksumInfo.isPresent()); dataChecksum = containerChecksumInfo.get().getContainerMerkleTree().getDataChecksum(); - verifyAllDataChecksumMatches(containerData, conf); + verifyAllDataChecksumsMatch(containerData, conf); } catch (IOException ex) { fail("Failed to read container checksum from disk", ex); } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java index c930320f62f9..273469c13b28 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java @@ -27,7 +27,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.GB; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.assertTreesSortedAndMatch; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.buildTestTree; -import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumMatches; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumsMatch; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.createBlockMetaData; import static org.apache.hadoop.ozone.container.common.impl.ContainerImplTestUtils.newContainerSet; import static org.assertj.core.api.Assertions.assertThat; @@ -681,7 +681,7 @@ public void testUpdateContainerChecksum(ContainerLayoutVersion layoutVersion) th // Initially, container should have no checksum information. assertEquals(0, containerData.getDataChecksum()); // Check all data checksums are updated correctly. - verifyAllDataChecksumMatches(containerData, conf); + verifyAllDataChecksumsMatch(containerData, conf); assertFalse(checksumManager.read(containerData).isPresent()); assertEquals(0, icrCount.get()); @@ -690,7 +690,7 @@ public void testUpdateContainerChecksum(ContainerLayoutVersion layoutVersion) th // Check ICR sent. The ICR sender verifies that the expected checksum is present in the report. assertEquals(1, icrCount.get()); // Check all data checksums are updated correctly. - verifyAllDataChecksumMatches(containerData, conf); + verifyAllDataChecksumsMatch(containerData, conf); // Check disk content. ContainerProtos.ContainerChecksumInfo checksumInfo = checksumManager.read(containerData).get(); assertTreesSortedAndMatch(treeWriter.toProto(), checksumInfo.getContainerMerkleTree()); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java index 42d589fffe2a..f36e54d8219d 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java @@ -46,7 +46,6 @@ import org.apache.commons.compress.archivers.tar.TarArchiveEntry; import org.apache.commons.compress.archivers.tar.TarArchiveInputStream; import org.apache.commons.compress.archivers.tar.TarArchiveOutputStream; -import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.utils.Archiver; @@ -56,9 +55,7 @@ import org.apache.hadoop.ozone.container.replication.CopyContainerCompression; import org.apache.ozone.test.SpyInputStream; import org.apache.ozone.test.SpyOutputStream; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -95,14 +92,14 @@ public class TestTarContainerPacker { private TarContainerPacker packer; - private static final Path SOURCE_CONTAINER_ROOT = - Paths.get("target/test/data/packer-source-dir"); + @TempDir + private Path sourceContainerRoot = Paths.get("target/test/data/packer-source-dir"); - private static final Path DEST_CONTAINER_ROOT = - Paths.get("target/test/data/packer-dest-dir"); + @TempDir + private Path destContainerRoot = Paths.get("target/test/data/packer-dest-dir"); - private static final Path TEMP_DIR = - Paths.get("target/test/data/packer-tmp-dir"); + @TempDir + private Path tempDir = Paths.get("target/test/data/packer-tmp-dir"); private static final AtomicInteger CONTAINER_ID = new AtomicInteger(1); @@ -133,34 +130,6 @@ public static List getLayoutAndCompression() { return parameterList; } - @BeforeAll - public static void init() throws IOException { - initDir(SOURCE_CONTAINER_ROOT); - initDir(DEST_CONTAINER_ROOT); - initDir(TEMP_DIR); - } - - @AfterAll - public static void cleanup() throws IOException { - FileUtils.deleteDirectory(SOURCE_CONTAINER_ROOT.toFile()); - FileUtils.deleteDirectory(DEST_CONTAINER_ROOT.toFile()); - FileUtils.deleteDirectory(TEMP_DIR.toFile()); - } - - @AfterEach - public void dirCleanUp() throws IOException { - FileUtils.cleanDirectory(SOURCE_CONTAINER_ROOT.toFile()); - FileUtils.cleanDirectory(DEST_CONTAINER_ROOT.toFile()); - FileUtils.cleanDirectory(TEMP_DIR.toFile()); - } - - private static void initDir(Path path) throws IOException { - if (path.toFile().exists()) { - FileUtils.deleteDirectory(path.toFile()); - } - Files.createDirectories(path); - } - private KeyValueContainerData createContainer(Path dir) throws IOException { return createContainer(dir, true, true); } @@ -200,7 +169,7 @@ public void pack(ContainerTestVersionInfo versionInfo, initTests(versionInfo, compression); //GIVEN KeyValueContainerData sourceContainerData = - createContainer(SOURCE_CONTAINER_ROOT, true, false); + createContainer(sourceContainerRoot, true, false); KeyValueContainer sourceContainer = new KeyValueContainer(sourceContainerData, conf); @@ -219,7 +188,7 @@ public void pack(ContainerTestVersionInfo versionInfo, //sample container descriptor file writeDescriptor(sourceContainer); - Path targetFile = TEMP_DIR.resolve("container.tar"); + Path targetFile = tempDir.resolve("container.tar"); //WHEN: pack it SpyOutputStream outputForPack = @@ -261,7 +230,7 @@ public void pack(ContainerTestVersionInfo versionInfo, inputForUnpackDescriptor.assertClosedExactlyOnce(); KeyValueContainerData destinationContainerData = - createContainer(DEST_CONTAINER_ROOT, false, false); + createContainer(destContainerRoot, false, false); KeyValueContainer destinationContainer = new KeyValueContainer(destinationContainerData, conf); @@ -271,7 +240,7 @@ public void pack(ContainerTestVersionInfo versionInfo, new SpyInputStream(newInputStream(targetFile)); String descriptor = new String( packer.unpackContainerData(destinationContainer, inputForUnpackData, - TEMP_DIR, DEST_CONTAINER_ROOT.resolve(String.valueOf( + tempDir, destContainerRoot.resolve(String.valueOf( destinationContainer.getContainerData().getContainerID()))), UTF_8); @@ -281,7 +250,6 @@ public void pack(ContainerTestVersionInfo versionInfo, assertExampleChunkFileIsGood( Paths.get(destinationContainerData.getChunksPath()), TEST_CHUNK_FILE_NAME); - Files.list(Paths.get(destinationContainerData.getMetadataPath())).forEach(System.out::println); assertEquals(sourceContainerData.getContainerID(), destinationContainerData.getContainerID()); assertTrue(ContainerChecksumTreeManager.hasContainerChecksumFile(destinationContainerData)); @@ -304,7 +272,7 @@ public void unpackContainerDataWithValidRelativeDbFilePath( initTests(versionInfo, compression); //GIVEN KeyValueContainerData sourceContainerData = - createContainer(SOURCE_CONTAINER_ROOT); + createContainer(sourceContainerRoot); String fileName = "sub/dir/" + TEST_DB_FILE_NAME; File file = writeDbFile(sourceContainerData, fileName); @@ -329,7 +297,7 @@ public void unpackContainerDataWithValidRelativeChunkFilePath( initTests(versionInfo, compression); //GIVEN KeyValueContainerData sourceContainerData = - createContainer(SOURCE_CONTAINER_ROOT); + createContainer(sourceContainerRoot); String fileName = "sub/dir/" + TEST_CHUNK_FILE_NAME; File file = writeChunkFile(sourceContainerData, fileName); @@ -353,7 +321,7 @@ public void unpackContainerDataWithInvalidRelativeDbFilePath( initTests(versionInfo, compression); //GIVEN KeyValueContainerData sourceContainerData = - createContainer(SOURCE_CONTAINER_ROOT); + createContainer(sourceContainerRoot); String fileName = "../db_file"; File file = writeDbFile(sourceContainerData, fileName); @@ -374,7 +342,7 @@ public void unpackContainerDataWithInvalidRelativeChunkFilePath( initTests(versionInfo, compression); //GIVEN KeyValueContainerData sourceContainerData = - createContainer(SOURCE_CONTAINER_ROOT); + createContainer(sourceContainerRoot); String fileName = "../chunk_file"; File file = writeChunkFile(sourceContainerData, fileName); @@ -389,10 +357,10 @@ public void unpackContainerDataWithInvalidRelativeChunkFilePath( private KeyValueContainerData unpackContainerData(File containerFile) throws IOException { try (InputStream input = newInputStream(containerFile.toPath())) { - KeyValueContainerData data = createContainer(DEST_CONTAINER_ROOT, false, true); + KeyValueContainerData data = createContainer(destContainerRoot, false, true); KeyValueContainer container = new KeyValueContainer(data, conf); - packer.unpackContainerData(container, input, TEMP_DIR, - DEST_CONTAINER_ROOT.resolve(String.valueOf(data.getContainerID()))); + packer.unpackContainerData(container, input, tempDir, + destContainerRoot.resolve(String.valueOf(data.getContainerID()))); return data; } } @@ -434,7 +402,7 @@ private File writeSingleFile(Path parentPath, String fileName, private File packContainerWithSingleFile(File file, String entryName) throws Exception { - File targetFile = TEMP_DIR.resolve("container.tar").toFile(); + File targetFile = tempDir.resolve("container.tar").toFile(); Path path = targetFile.toPath(); try (TarArchiveOutputStream archive = new TarArchiveOutputStream(packer.compress(newOutputStream(path)))) { archive.setBigNumberMode(TarArchiveOutputStream.BIGNUMBER_POSIX); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java index fa019df57f1e..95475651d014 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/impl/TestFilePerBlockStrategy.java @@ -22,7 +22,7 @@ import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_DATANODE_DIR_KEY; import static org.apache.hadoop.ozone.container.ContainerTestHelper.getChunk; import static org.apache.hadoop.ozone.container.ContainerTestHelper.setDataChecksum; -import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumMatches; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumsMatch; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.WRITE_STAGE; import static org.apache.hadoop.ozone.container.common.impl.ContainerImplTestUtils.newContainerSet; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -255,7 +255,7 @@ public void testPutBlockForClosedContainer() throws IOException { assertEquals(1L, containerData.getBlockCommitSequenceId()); assertEquals(1L, containerData.getBlockCount()); assertEquals(20L, containerData.getBytesUsed()); - verifyAllDataChecksumMatches(containerData, conf); + verifyAllDataChecksumsMatch(containerData, conf); try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { long localID = putBlockData.getLocalID(); @@ -279,7 +279,7 @@ public void testPutBlockForClosedContainer() throws IOException { assertEquals(1L, containerData.getBlockCount()); assertEquals(40L, containerData.getBytesUsed()); assertNotEquals(previousDataChecksum, containerData.getDataChecksum()); - verifyAllDataChecksumMatches(containerData, conf); + verifyAllDataChecksumsMatch(containerData, conf); try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { long localID = putBlockData.getLocalID(); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java index 1b9b658556c2..a5ff67afe00b 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestBackgroundContainerDataScannerIntegration.java @@ -20,7 +20,7 @@ import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.CLOSED; import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.UNHEALTHY; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.readChecksumFile; -import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumMatches; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumsMatch; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -80,7 +80,6 @@ void testCorruptionDetected(TestContainerCorruptions corruption) throws Exception { pauseScanner(); - OzoneConfiguration conf = new OzoneConfiguration(); long containerID = writeDataThenCloseContainer(); // Container corruption has not yet been introduced. Container container = getDnContainer(containerID); @@ -115,7 +114,7 @@ void testCorruptionDetected(TestContainerCorruptions corruption) ContainerProtos.ContainerChecksumInfo updatedChecksumInfo = readChecksumFile(container.getContainerData()); assertEquals(newReportedDataChecksum, updatedChecksumInfo.getContainerMerkleTree().getDataChecksum()); KeyValueContainerData containerData = (KeyValueContainerData) container.getContainerData(); - verifyAllDataChecksumMatches(containerData, conf); + verifyAllDataChecksumsMatch(containerData, getConf()); } if (corruption == TestContainerCorruptions.TRUNCATED_BLOCK || diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestContainerScannerIntegrationAbstract.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestContainerScannerIntegrationAbstract.java index 2584138c126b..399052e79ef1 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestContainerScannerIntegrationAbstract.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestContainerScannerIntegrationAbstract.java @@ -205,4 +205,8 @@ private OzoneOutputStream createKey(String keyName) throws Exception { return TestHelper.createKey( keyName, RATIS, ONE, 0, store, volumeName, bucketName); } + + protected OzoneConfiguration getConf() { + return cluster.getConf(); + } } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java index 156c876bed59..f4f87b158be7 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/dn/scanner/TestOnDemandContainerDataScannerIntegration.java @@ -20,7 +20,7 @@ import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.CLOSED; import static org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReplicaProto.State.UNHEALTHY; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.readChecksumFile; -import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumMatches; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumsMatch; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -100,7 +100,6 @@ static void init() throws Exception { void testCorruptionDetected(TestContainerCorruptions corruption) throws Exception { String keyName = "testKey"; - OzoneConfiguration conf = new OzoneConfiguration(); long containerID = writeDataThenCloseContainer(keyName); // Container corruption has not yet been introduced. Container container = getDnContainer(containerID); @@ -139,7 +138,7 @@ void testCorruptionDetected(TestContainerCorruptions corruption) ContainerProtos.ContainerChecksumInfo updatedChecksumInfo = readChecksumFile(container.getContainerData()); assertEquals(newReportedDataChecksum, updatedChecksumInfo.getContainerMerkleTree().getDataChecksum()); KeyValueContainerData containerData = (KeyValueContainerData) container.getContainerData(); - verifyAllDataChecksumMatches(containerData, conf); + verifyAllDataChecksumsMatch(containerData, getConf()); } } } From f0a7ab381b8bd59582fcd1beade7c03ff1cb0f9f Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Tue, 8 Jul 2025 10:36:00 -0700 Subject: [PATCH 04/10] Fix test. --- .../hadoop/ozone/container/keyvalue/TestKeyValueHandler.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java index 273469c13b28..4a910060d14f 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java @@ -680,8 +680,6 @@ public void testUpdateContainerChecksum(ContainerLayoutVersion layoutVersion) th // Initially, container should have no checksum information. assertEquals(0, containerData.getDataChecksum()); - // Check all data checksums are updated correctly. - verifyAllDataChecksumsMatch(containerData, conf); assertFalse(checksumManager.read(containerData).isPresent()); assertEquals(0, icrCount.get()); From 483cee5c3f146f62158bfdfbaacd89d2afec274a Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Tue, 8 Jul 2025 11:19:32 -0700 Subject: [PATCH 05/10] Update TempDir reference. --- .../ozone/container/keyvalue/TestTarContainerPacker.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java index f36e54d8219d..c20ef0dd49ad 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java @@ -93,13 +93,13 @@ public class TestTarContainerPacker { private TarContainerPacker packer; @TempDir - private Path sourceContainerRoot = Paths.get("target/test/data/packer-source-dir"); + private Path sourceContainerRoot; @TempDir - private Path destContainerRoot = Paths.get("target/test/data/packer-dest-dir"); + private Path destContainerRoot; @TempDir - private Path tempDir = Paths.get("target/test/data/packer-tmp-dir"); + private Path tempDir; private static final AtomicInteger CONTAINER_ID = new AtomicInteger(1); From 5c0b0dc431a809bed269c835a9c1ea5621bba73f Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Wed, 23 Jul 2025 14:30:24 -0700 Subject: [PATCH 06/10] Add new tests and address review comments --- .../ContainerChecksumTreeManager.java | 6 +- .../helpers/KeyValueContainerUtil.java | 4 +- .../ozoneimpl/TestContainerReader.java | 124 ++++++++++++++++++ 3 files changed, 129 insertions(+), 5 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 fa1924143142..0f14a1724900 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 @@ -24,7 +24,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.util.concurrent.Striped; -import jakarta.annotation.Nullable; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -421,15 +420,16 @@ public ByteString getContainerChecksumInfo(KeyValueContainerData data) throws IO * Callers are not required to hold a lock while calling this since writes are done to a tmp file and atomically * swapped into place. */ - public static @Nullable ContainerProtos.ContainerChecksumInfo readChecksumInfo(ContainerData data) + public static ContainerProtos.ContainerChecksumInfo readChecksumInfo(ContainerData data) throws IOException { long containerID = data.getContainerID(); File checksumFile = getContainerChecksumFile(data); try { if (!checksumFile.exists()) { LOG.debug("No checksum file currently exists for container {} at the path {}", containerID, checksumFile); - return null; + return ContainerProtos.ContainerChecksumInfo.newBuilder().build(); } + try (InputStream inStream = Files.newInputStream(checksumFile.toPath())) { return ContainerProtos.ContainerChecksumInfo.parseFrom(inStream); } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java index 41ee000785bc..d8dc13daf5f0 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java @@ -294,14 +294,14 @@ private static void loadAndSetContainerDataChecksum(KeyValueContainerData kvCont } Long containerDataChecksum = metadataTable.get(kvContainerData.getContainerDataChecksumKey()); - if (containerDataChecksum != null) { + if (containerDataChecksum != null && kvContainerData.needsDataChecksum()) { kvContainerData.setDataChecksum(containerDataChecksum); return; } try { ContainerChecksumInfo containerChecksumInfo = ContainerChecksumTreeManager.readChecksumInfo(kvContainerData); - if (containerChecksumInfo != null) { + if (containerChecksumInfo != null && kvContainerData.needsDataChecksum()) { containerDataChecksum = containerChecksumInfo.getContainerMerkleTree().getDataChecksum(); kvContainerData.setDataChecksum(containerDataChecksum); metadataTable.put(kvContainerData.getContainerDataChecksumKey(), containerDataChecksum); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java index ec5c6743e729..10b087d6b291 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java @@ -20,12 +20,14 @@ import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.DELETED; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.RECOVERING; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.UNHEALTHY; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumsMatch; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.createDbInstancesForTestIfNeeded; import static org.apache.hadoop.ozone.container.common.impl.ContainerImplTestUtils.newContainerSet; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.anyList; @@ -39,8 +41,10 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.UUID; +import java.util.stream.Collectors; import org.apache.hadoop.conf.StorageUnit; import org.apache.hadoop.hdds.client.BlockID; import org.apache.hadoop.hdds.conf.OzoneConfiguration; @@ -49,6 +53,9 @@ import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.ozone.OzoneConfigKeys; import org.apache.hadoop.ozone.OzoneConsts; +import org.apache.hadoop.ozone.container.checksum.ContainerChecksumTreeManager; +import org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils; +import org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeWriter; 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.impl.ContainerLayoutVersion; @@ -609,6 +616,123 @@ public void testMarkedDeletedContainerCleared( } } + @ContainerTestVersionInfo.ContainerTest + public void testContainerLoadingWithMerkleTreePresent(ContainerTestVersionInfo versionInfo) + throws Exception { + setLayoutAndSchemaVersion(versionInfo); + setup(versionInfo); + + // Create a container with blocks and write MerkleTree + KeyValueContainerData containerData = createContainerWithBlocks(10L); + ContainerMerkleTreeWriter treeWriter = ContainerMerkleTreeTestUtils.buildTestTree(conf); + ContainerChecksumTreeManager checksumManager = new ContainerChecksumTreeManager(conf); + List deletedBlockIds = Arrays.asList(1L, 2L, 3L); + checksumManager.markBlocksAsDeleted(containerData, deletedBlockIds); + ContainerProtos.ContainerChecksumInfo checksumInfo = + checksumManager.writeContainerDataTree(containerData, treeWriter); + long expectedDataChecksum = checksumInfo.getContainerMerkleTree().getDataChecksum(); + + // Test container loading + ContainerCache.getInstance(conf).shutdownCache(); + ContainerReader containerReader = new ContainerReader(volumeSet, hddsVolume, containerSet, conf, true); + containerReader.run(); + + // Verify container was loaded successfully and data checksum is set + Container loadedContainer = containerSet.getContainer(10L); + assertNotNull(loadedContainer); + KeyValueContainerData loadedData = (KeyValueContainerData) loadedContainer.getContainerData(); + assertNotSame(containerData, loadedData); + assertEquals(expectedDataChecksum, loadedData.getDataChecksum()); + ContainerProtos.ContainerChecksumInfo loadedChecksumInfo = + ContainerChecksumTreeManager.readChecksumInfo(loadedData); + verifyAllDataChecksumsMatch(loadedData, conf); + + // Verify the deleted block IDs match what we set + List loadedDeletedBlockIds = loadedChecksumInfo.getDeletedBlocksList().stream() + .map(ContainerProtos.BlockMerkleTree::getBlockID) + .sorted() + .collect(Collectors.toList()); + assertEquals(3, loadedChecksumInfo.getDeletedBlocksCount()); + assertEquals(deletedBlockIds, loadedDeletedBlockIds); + } + + @ContainerTestVersionInfo.ContainerTest + public void testContainerLoadingWithMerkleTreeFallbackToRocksDB(ContainerTestVersionInfo versionInfo) + throws Exception { + setLayoutAndSchemaVersion(versionInfo); + setup(versionInfo); + + KeyValueContainerData containerData = createContainerWithBlocks(11L); + ContainerMerkleTreeWriter treeWriter = ContainerMerkleTreeTestUtils.buildTestTree(conf); + ContainerChecksumTreeManager checksumManager = new ContainerChecksumTreeManager(conf); + ContainerProtos.ContainerChecksumInfo checksumInfo = + checksumManager.writeContainerDataTree(containerData, treeWriter); + long dataChecksum = checksumInfo.getContainerMerkleTree().getDataChecksum(); + + // Verify no checksum in RocksDB initially + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + assertNull(dbDataChecksum); + } + ContainerCache.getInstance(conf).shutdownCache(); + + // Test container loading - should read from MerkleTree and store in RocksDB + ContainerReader containerReader = new ContainerReader(volumeSet, hddsVolume, containerSet, conf, true); + containerReader.run(); + + // Verify container uses checksum from MerkleTree + Container loadedContainer = containerSet.getContainer(11L); + assertNotNull(loadedContainer); + KeyValueContainerData loadedData = (KeyValueContainerData) loadedContainer.getContainerData(); + assertNotSame(containerData, loadedData); + assertEquals(dataChecksum, loadedData.getDataChecksum()); + + // Verify checksum was stored in RocksDB as fallback + verifyAllDataChecksumsMatch(loadedData, conf); + } + + @ContainerTestVersionInfo.ContainerTest + public void testContainerLoadingWithNoChecksumAnywhere(ContainerTestVersionInfo versionInfo) + throws Exception { + setLayoutAndSchemaVersion(versionInfo); + setup(versionInfo); + + KeyValueContainerData containerData = createContainerWithBlocks(12L); + // Verify no checksum in RocksDB + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + assertNull(dbDataChecksum); + } + + File checksumFile = ContainerChecksumTreeManager.getContainerChecksumFile(containerData); + assertFalse(checksumFile.exists()); + + // Test container loading - should default to 0 + ContainerCache.getInstance(conf).shutdownCache(); + ContainerReader containerReader = new ContainerReader(volumeSet, hddsVolume, containerSet, conf, true); + containerReader.run(); + + // Verify container loads with default checksum of 0 + Container loadedContainer = containerSet.getContainer(12L); + assertNotNull(loadedContainer); + KeyValueContainerData loadedData = (KeyValueContainerData) loadedContainer.getContainerData(); + assertNotSame(containerData, loadedData); + assertEquals(0L, loadedData.getDataChecksum()); + + // Verify 0 checksum was stored in RocksDB + verifyAllDataChecksumsMatch(loadedData, conf); + } + + private KeyValueContainerData createContainerWithBlocks(long containerId) throws Exception { + KeyValueContainerData containerData = new KeyValueContainerData(containerId, layout, + (long) StorageUnit.GB.toBytes(5), UUID.randomUUID().toString(), datanodeId.toString()); + containerData.setState(ContainerProtos.ContainerDataProto.State.CLOSED); + KeyValueContainer container = new KeyValueContainer(containerData, conf); + container.create(volumeSet, volumeChoosingPolicy, clusterId); + addBlocks(container, true); + return containerData; + } + private long addDbEntry(KeyValueContainerData containerData) throws Exception { try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { From 7811e5f538f8369c61fe86a3bc8caab9dfa5a6f0 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Mon, 28 Jul 2025 12:41:41 -0700 Subject: [PATCH 07/10] Address review comments. --- .../helpers/KeyValueContainerUtil.java | 3 +- .../keyvalue/TestKeyValueContainer.java | 59 +++++++++++++++++++ .../ozoneimpl/TestContainerReader.java | 51 +++++++++++++++- 3 files changed, 110 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java index d8dc13daf5f0..d8ebd0472cf0 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java @@ -301,7 +301,8 @@ private static void loadAndSetContainerDataChecksum(KeyValueContainerData kvCont try { ContainerChecksumInfo containerChecksumInfo = ContainerChecksumTreeManager.readChecksumInfo(kvContainerData); - if (containerChecksumInfo != null && kvContainerData.needsDataChecksum()) { + if (containerChecksumInfo != null && containerChecksumInfo.hasContainerMerkleTree() + && kvContainerData.needsDataChecksum()) { containerDataChecksum = containerChecksumInfo.getContainerMerkleTree().getDataChecksum(); kvContainerData.setDataChecksum(containerDataChecksum); metadataTable.put(kvContainerData.getContainerDataChecksumKey(), containerDataChecksum); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java index 7ba72f937efd..1328852b23f7 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java @@ -73,6 +73,7 @@ import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.container.ContainerTestHelper; +import org.apache.hadoop.ozone.container.checksum.ContainerChecksumTreeManager; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.impl.ContainerDataYaml; import org.apache.hadoop.ozone.container.common.impl.ContainerLayoutVersion; @@ -291,6 +292,56 @@ public void testEmptyContainerImportExport( checkContainerFilesPresent(data, 0); } + @ContainerTestVersionInfo.ContainerTest + public void testEmptyMerkleTreeImportExport(ContainerTestVersionInfo versionInfo) throws Exception { + init(versionInfo); + createContainer(); + closeContainer(); + + KeyValueContainerData data = keyValueContainer.getContainerData(); + // Create an empty checksum file that exists but has no valid merkle tree + File checksumFile = ContainerChecksumTreeManager.getContainerChecksumFile(data); + ContainerProtos.ContainerChecksumInfo emptyContainerInfo = ContainerProtos.ContainerChecksumInfo + .newBuilder().build(); + try (OutputStream tmpOutputStream = Files.newOutputStream(checksumFile.toPath())) { + emptyContainerInfo.writeTo(tmpOutputStream); + } + + // Check state of original container. + checkContainerFilesPresent(data, 0); + + //destination path + File exportTar = Files.createFile( + folder.toPath().resolve("export.tar")).toFile(); + TarContainerPacker packer = new TarContainerPacker(NO_COMPRESSION); + //export the container + try (OutputStream fos = Files.newOutputStream(exportTar.toPath())) { + keyValueContainer.exportContainerData(fos, packer); + } + + KeyValueContainerUtil.removeContainer( + keyValueContainer.getContainerData(), CONF); + keyValueContainer.delete(); + + // import container. + try (InputStream fis = Files.newInputStream(exportTar.toPath())) { + keyValueContainer.importContainerData(fis, packer); + } + + // Make sure empty chunks dir was unpacked. + checkContainerFilesPresent(data, 0); + data = keyValueContainer.getContainerData(); + ContainerProtos.ContainerChecksumInfo checksumInfo = ContainerChecksumTreeManager.readChecksumInfo(data); + assertFalse(checksumInfo.hasContainerMerkleTree()); + // The import should not fail and the checksum should be 0 + assertEquals(0, data.getDataChecksum()); + // The checksum is not stored in rocksDB as the container merkle tree doesn't exist. + try (DBHandle dbHandle = BlockUtils.getDB(data, new OzoneConfiguration())) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(data.getContainerDataChecksumKey()); + assertNull(dbDataChecksum); + } + } + @ContainerTestVersionInfo.ContainerTest public void testUnhealthyContainerImportExport( ContainerTestVersionInfo versionInfo) throws Exception { @@ -387,6 +438,14 @@ public void testContainerImportExport(ContainerTestVersionInfo versionInfo) containerData.getMaxSize()); assertEquals(keyValueContainerData.getBytesUsed(), containerData.getBytesUsed()); + assertEquals(0L, keyValueContainerData.getDataChecksum()); + // The checksum is not stored in rocksDB as the checksum file doesn't exist. + try (DBHandle dbHandle = BlockUtils.getDB(keyValueContainerData, new OzoneConfiguration())) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get( + keyValueContainerData.getContainerDataChecksumKey()); + assertNull(dbDataChecksum); + } + assertFalse(ContainerChecksumTreeManager.getContainerChecksumFile(containerData).exists()); assertNotNull(containerData.getContainerFileChecksum()); assertNotEquals(containerData.ZERO_CHECKSUM, container.getContainerData().getContainerFileChecksum()); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java index 10b087d6b291..431a71a03ff5 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java @@ -37,6 +37,7 @@ import java.io.File; import java.io.IOException; +import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -719,8 +720,54 @@ public void testContainerLoadingWithNoChecksumAnywhere(ContainerTestVersionInfo assertNotSame(containerData, loadedData); assertEquals(0L, loadedData.getDataChecksum()); - // Verify 0 checksum was stored in RocksDB - verifyAllDataChecksumsMatch(loadedData, conf); + // The checksum is not stored in rocksDB as the container merkle tree doesn't exist. + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + assertNull(dbDataChecksum); + } + } + + @ContainerTestVersionInfo.ContainerTest + public void testContainerLoadingWithoutMerkleTree(ContainerTestVersionInfo versionInfo) + throws Exception { + setLayoutAndSchemaVersion(versionInfo); + setup(versionInfo); + + KeyValueContainerData containerData = createContainerWithBlocks(13L); + + // Create an empty checksum file that exists but has no valid merkle tree + File checksumFile = ContainerChecksumTreeManager.getContainerChecksumFile(containerData); + ContainerProtos.ContainerChecksumInfo emptyContainerInfo = ContainerProtos.ContainerChecksumInfo + .newBuilder().build(); + try (OutputStream tmpOutputStream = Files.newOutputStream(checksumFile.toPath())) { + emptyContainerInfo.writeTo(tmpOutputStream); + } + assertTrue(checksumFile.exists()); + + // Verify no checksum in RocksDB initially + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + assertNull(dbDataChecksum); + } + + ContainerCache.getInstance(conf).shutdownCache(); + + // Test container loading - should handle corrupted file gracefully and default to 0 + ContainerReader containerReader = new ContainerReader(volumeSet, hddsVolume, containerSet, conf, true); + containerReader.run(); + + // Verify container loads with default checksum of 0 when file is corrupted + Container loadedContainer = containerSet.getContainer(13L); + assertNotNull(loadedContainer); + KeyValueContainerData loadedData = (KeyValueContainerData) loadedContainer.getContainerData(); + assertNotSame(containerData, loadedData); + assertEquals(0L, loadedData.getDataChecksum()); + + // The checksum is not stored in rocksDB as the container merkle tree doesn't exist. + try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { + Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + assertNull(dbDataChecksum); + } } private KeyValueContainerData createContainerWithBlocks(long containerId) throws Exception { From 3f9cea6d3aacae596542c74b0b702484fbc6dbb6 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Wed, 30 Jul 2025 13:02:09 -0700 Subject: [PATCH 08/10] Address review comments. --- .../helpers/KeyValueContainerUtil.java | 12 ++--- .../ozoneimpl/TestContainerReader.java | 54 +++++++++---------- 2 files changed, 31 insertions(+), 35 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java index d8ebd0472cf0..70e480241029 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java @@ -293,13 +293,13 @@ private static void loadAndSetContainerDataChecksum(KeyValueContainerData kvCont return; } - Long containerDataChecksum = metadataTable.get(kvContainerData.getContainerDataChecksumKey()); - if (containerDataChecksum != null && kvContainerData.needsDataChecksum()) { - kvContainerData.setDataChecksum(containerDataChecksum); - return; - } - try { + Long containerDataChecksum = metadataTable.get(kvContainerData.getContainerDataChecksumKey()); + if (containerDataChecksum != null && kvContainerData.needsDataChecksum()) { + kvContainerData.setDataChecksum(containerDataChecksum); + return; + } + ContainerChecksumInfo containerChecksumInfo = ContainerChecksumTreeManager.readChecksumInfo(kvContainerData); if (containerChecksumInfo != null && containerChecksumInfo.hasContainerMerkleTree() && kvContainerData.needsDataChecksum()) { diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java index 431a71a03ff5..bf9143270456 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java @@ -22,6 +22,7 @@ import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.UNHEALTHY; import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumsMatch; import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.createDbInstancesForTestIfNeeded; +import static org.apache.hadoop.ozone.container.common.ContainerTestUtils.getKeyValueHandler; import static org.apache.hadoop.ozone.container.common.impl.ContainerImplTestUtils.newContainerSet; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -37,7 +38,6 @@ import java.io.File; import java.io.IOException; -import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -74,6 +74,7 @@ import org.apache.hadoop.ozone.container.keyvalue.ContainerTestVersionInfo; import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainer; import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; +import org.apache.hadoop.ozone.container.keyvalue.KeyValueHandler; import org.apache.hadoop.ozone.container.keyvalue.helpers.BlockUtils; import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaThreeImpl; import org.apache.hadoop.util.Time; @@ -100,6 +101,7 @@ public class TestContainerReader { private ContainerLayoutVersion layout; private String schemaVersion; + private KeyValueHandler keyValueHandler; @TempDir private Path tempDir; @@ -148,6 +150,7 @@ private void setup(ContainerTestVersionInfo versionInfo) throws Exception { // so it does not affect the ContainerReader, which avoids using the cache // at startup for performance reasons. ContainerCache.getInstance(conf).shutdownCache(); + keyValueHandler = getKeyValueHandler(conf, UUID.randomUUID().toString(), containerSet, volumeSet); } @AfterEach @@ -624,14 +627,14 @@ public void testContainerLoadingWithMerkleTreePresent(ContainerTestVersionInfo v setup(versionInfo); // Create a container with blocks and write MerkleTree - KeyValueContainerData containerData = createContainerWithBlocks(10L); + KeyValueContainer container = createContainer(10L); + KeyValueContainerData containerData = container.getContainerData(); ContainerMerkleTreeWriter treeWriter = ContainerMerkleTreeTestUtils.buildTestTree(conf); - ContainerChecksumTreeManager checksumManager = new ContainerChecksumTreeManager(conf); + ContainerChecksumTreeManager checksumManager = keyValueHandler.getChecksumManager(); List deletedBlockIds = Arrays.asList(1L, 2L, 3L); checksumManager.markBlocksAsDeleted(containerData, deletedBlockIds); - ContainerProtos.ContainerChecksumInfo checksumInfo = - checksumManager.writeContainerDataTree(containerData, treeWriter); - long expectedDataChecksum = checksumInfo.getContainerMerkleTree().getDataChecksum(); + keyValueHandler.updateContainerChecksum(container, treeWriter); + long expectedDataChecksum = checksumManager.read(containerData).getContainerMerkleTree().getDataChecksum(); // Test container loading ContainerCache.getInstance(conf).shutdownCache(); @@ -663,7 +666,8 @@ public void testContainerLoadingWithMerkleTreeFallbackToRocksDB(ContainerTestVer setLayoutAndSchemaVersion(versionInfo); setup(versionInfo); - KeyValueContainerData containerData = createContainerWithBlocks(11L); + KeyValueContainer container = createContainer(11L); + KeyValueContainerData containerData = container.getContainerData(); ContainerMerkleTreeWriter treeWriter = ContainerMerkleTreeTestUtils.buildTestTree(conf); ContainerChecksumTreeManager checksumManager = new ContainerChecksumTreeManager(conf); ContainerProtos.ContainerChecksumInfo checksumInfo = @@ -698,7 +702,8 @@ public void testContainerLoadingWithNoChecksumAnywhere(ContainerTestVersionInfo setLayoutAndSchemaVersion(versionInfo); setup(versionInfo); - KeyValueContainerData containerData = createContainerWithBlocks(12L); + KeyValueContainer container = createContainer(12L); + KeyValueContainerData containerData = container.getContainerData(); // Verify no checksum in RocksDB try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); @@ -720,7 +725,7 @@ public void testContainerLoadingWithNoChecksumAnywhere(ContainerTestVersionInfo assertNotSame(containerData, loadedData); assertEquals(0L, loadedData.getDataChecksum()); - // The checksum is not stored in rocksDB as the container merkle tree doesn't exist. + // The checksum is not stored in rocksDB as the checksum file doesn't exist. try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); assertNull(dbDataChecksum); @@ -733,16 +738,12 @@ public void testContainerLoadingWithoutMerkleTree(ContainerTestVersionInfo versi setLayoutAndSchemaVersion(versionInfo); setup(versionInfo); - KeyValueContainerData containerData = createContainerWithBlocks(13L); - + KeyValueContainer container = createContainer(13L); + KeyValueContainerData containerData = container.getContainerData(); + ContainerMerkleTreeWriter treeWriter = new ContainerMerkleTreeWriter(); + keyValueHandler.updateContainerChecksum(container, treeWriter); // Create an empty checksum file that exists but has no valid merkle tree - File checksumFile = ContainerChecksumTreeManager.getContainerChecksumFile(containerData); - ContainerProtos.ContainerChecksumInfo emptyContainerInfo = ContainerProtos.ContainerChecksumInfo - .newBuilder().build(); - try (OutputStream tmpOutputStream = Files.newOutputStream(checksumFile.toPath())) { - emptyContainerInfo.writeTo(tmpOutputStream); - } - assertTrue(checksumFile.exists()); + assertTrue(ContainerChecksumTreeManager.getContainerChecksumFile(containerData).exists()); // Verify no checksum in RocksDB initially try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { @@ -752,32 +753,27 @@ public void testContainerLoadingWithoutMerkleTree(ContainerTestVersionInfo versi ContainerCache.getInstance(conf).shutdownCache(); - // Test container loading - should handle corrupted file gracefully and default to 0 + // Test container loading - should handle when checksum file is present without the container merkle tree and + // default to 0. ContainerReader containerReader = new ContainerReader(volumeSet, hddsVolume, containerSet, conf, true); containerReader.run(); - // Verify container loads with default checksum of 0 when file is corrupted + // Verify container loads with default checksum of 0 when checksum file doesn't have merkle tree Container loadedContainer = containerSet.getContainer(13L); assertNotNull(loadedContainer); KeyValueContainerData loadedData = (KeyValueContainerData) loadedContainer.getContainerData(); assertNotSame(containerData, loadedData); assertEquals(0L, loadedData.getDataChecksum()); - - // The checksum is not stored in rocksDB as the container merkle tree doesn't exist. - try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { - Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); - assertNull(dbDataChecksum); - } + verifyAllDataChecksumsMatch(loadedData, conf); } - private KeyValueContainerData createContainerWithBlocks(long containerId) throws Exception { + private KeyValueContainer createContainer(long containerId) throws Exception { KeyValueContainerData containerData = new KeyValueContainerData(containerId, layout, (long) StorageUnit.GB.toBytes(5), UUID.randomUUID().toString(), datanodeId.toString()); containerData.setState(ContainerProtos.ContainerDataProto.State.CLOSED); KeyValueContainer container = new KeyValueContainer(containerData, conf); container.create(volumeSet, volumeChoosingPolicy, clusterId); - addBlocks(container, true); - return containerData; + return container; } private long addDbEntry(KeyValueContainerData containerData) From e104acf60ee731d2fdffe48f740cc0288aad6ffe Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Thu, 31 Jul 2025 12:03:12 -0700 Subject: [PATCH 09/10] Address review comments. --- .../ContainerMerkleTreeTestUtils.java | 20 +++++++++++++++---- .../ozoneimpl/TestContainerReader.java | 5 +---- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java index 95e7aeaa9f09..0f7794bac4cb 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/checksum/ContainerMerkleTreeTestUtils.java @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.container.checksum; import static org.apache.hadoop.ozone.container.checksum.ContainerChecksumTreeManager.getContainerChecksumFile; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -366,11 +367,22 @@ public static void verifyAllDataChecksumsMatch(KeyValueContainerData containerDa .readChecksumInfo(containerData); assertNotNull(containerChecksumInfo); long dataChecksum = containerChecksumInfo.getContainerMerkleTree().getDataChecksum(); - assertEquals(containerData.getDataChecksum(), dataChecksum, "In-memory data checksum should match " + - "the one in the checksum file."); + Long dbDataChecksum; try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { - Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); - assertEquals(containerData.getDataChecksum(), dbDataChecksum, "DB should have the updated data checksum."); + dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); + } + + if (containerData.getDataChecksum() == 0) { + assertEquals(containerData.getDataChecksum(), dataChecksum); + // RocksDB checksum can be null if the file doesn't exist or when the file is created by + // the block deleting service. 0 checksum will be stored when the container is loaded without + // merkle tree. + assertThat(dbDataChecksum).isIn(0L, null); + } else { + // In-Memory, Container Merkle Tree file, RocksDB checksum should be equal + assertEquals(containerData.getDataChecksum(), dataChecksum, "In-memory data checksum should match " + + "the one in the checksum file."); + assertEquals(dbDataChecksum, dataChecksum); } } } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java index bf9143270456..69415972d008 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/ozoneimpl/TestContainerReader.java @@ -726,10 +726,7 @@ public void testContainerLoadingWithNoChecksumAnywhere(ContainerTestVersionInfo assertEquals(0L, loadedData.getDataChecksum()); // The checksum is not stored in rocksDB as the checksum file doesn't exist. - try (DBHandle dbHandle = BlockUtils.getDB(containerData, conf)) { - Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(containerData.getContainerDataChecksumKey()); - assertNull(dbDataChecksum); - } + verifyAllDataChecksumsMatch(loadedData, conf); } @ContainerTestVersionInfo.ContainerTest From ff568fd0375bc527d13ca2550bb71cc91ccb6e62 Mon Sep 17 00:00:00 2001 From: Aswin Shakil Balasubramanian Date: Tue, 5 Aug 2025 12:47:30 -0700 Subject: [PATCH 10/10] Modify Rocks DB open for V1/V2 and Update TestKeyValueContainer tests. --- .../helpers/KeyValueContainerUtil.java | 10 +++---- .../keyvalue/TestKeyValueContainer.java | 30 +++++++++++-------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java index 70e480241029..28b1711e980d 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java @@ -259,10 +259,10 @@ public static void parseKVContainerData(KeyValueContainerData kvContainerData, DatanodeStore store = null; try { try { - boolean readOnly = ContainerInspectorUtil.isReadOnly( - ContainerProtos.ContainerType.KeyValueContainer); - store = BlockUtils.getUncachedDatanodeStore( - kvContainerData, config, readOnly); + // Open RocksDB in write mode, as it is required for container checksum updates and inspector repair operations. + // The method KeyValueContainerMetadataInspector.buildErrorAndRepair will determine if write access to the DB + // is permitted based on the mode. + store = BlockUtils.getUncachedDatanodeStore(kvContainerData, config, false); } catch (IOException e) { // If an exception is thrown, then it may indicate the RocksDB is // already open in the container cache. As this code is only executed at @@ -288,7 +288,7 @@ public static void parseKVContainerData(KeyValueContainerData kvContainerData, } private static void loadAndSetContainerDataChecksum(KeyValueContainerData kvContainerData, - Table metadataTable) throws IOException { + Table metadataTable) { if (kvContainerData.isOpen()) { return; } diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java index 1328852b23f7..28c118b517f6 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java @@ -20,6 +20,8 @@ import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_DB_PROFILE; import static org.apache.hadoop.ozone.OzoneConsts.SCHEMA_V2; import static org.apache.hadoop.ozone.OzoneConsts.SCHEMA_V3; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.buildTestTree; +import static org.apache.hadoop.ozone.container.checksum.ContainerMerkleTreeTestUtils.verifyAllDataChecksumsMatch; import static org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration.CONTAINER_SCHEMA_V3_ENABLED; import static org.apache.hadoop.ozone.container.keyvalue.helpers.KeyValueContainerUtil.isSameSchemaVersion; import static org.apache.hadoop.ozone.container.replication.CopyContainerCompression.NO_COMPRESSION; @@ -336,10 +338,7 @@ public void testEmptyMerkleTreeImportExport(ContainerTestVersionInfo versionInfo // The import should not fail and the checksum should be 0 assertEquals(0, data.getDataChecksum()); // The checksum is not stored in rocksDB as the container merkle tree doesn't exist. - try (DBHandle dbHandle = BlockUtils.getDB(data, new OzoneConfiguration())) { - Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get(data.getContainerDataChecksumKey()); - assertNull(dbDataChecksum); - } + verifyAllDataChecksumsMatch(data, CONF); } @ContainerTestVersionInfo.ContainerTest @@ -391,6 +390,18 @@ public void testContainerImportExport(ContainerTestVersionInfo versionInfo) closeContainer(); populate(numberOfKeysToWrite); + // Create merkle tree and set data checksum to simulate actual key value container. + File checksumFile = ContainerChecksumTreeManager.getContainerChecksumFile( + keyValueContainer.getContainerData()); + ContainerProtos.ContainerMerkleTree containerMerkleTreeWriterProto = buildTestTree(CONF).toProto(); + keyValueContainerData.setDataChecksum(containerMerkleTreeWriterProto.getDataChecksum()); + ContainerProtos.ContainerChecksumInfo containerInfo = ContainerProtos.ContainerChecksumInfo.newBuilder() + .setContainerID(containerId) + .setContainerMerkleTree(containerMerkleTreeWriterProto).build(); + try (OutputStream tmpOutputStream = Files.newOutputStream(checksumFile.toPath())) { + containerInfo.writeTo(tmpOutputStream); + } + //destination path File folderToExport = Files.createFile( folder.toPath().resolve("export.tar")).toFile(); @@ -438,14 +449,9 @@ public void testContainerImportExport(ContainerTestVersionInfo versionInfo) containerData.getMaxSize()); assertEquals(keyValueContainerData.getBytesUsed(), containerData.getBytesUsed()); - assertEquals(0L, keyValueContainerData.getDataChecksum()); - // The checksum is not stored in rocksDB as the checksum file doesn't exist. - try (DBHandle dbHandle = BlockUtils.getDB(keyValueContainerData, new OzoneConfiguration())) { - Long dbDataChecksum = dbHandle.getStore().getMetadataTable().get( - keyValueContainerData.getContainerDataChecksumKey()); - assertNull(dbDataChecksum); - } - assertFalse(ContainerChecksumTreeManager.getContainerChecksumFile(containerData).exists()); + assertEquals(keyValueContainerData.getDataChecksum(), containerData.getDataChecksum()); + verifyAllDataChecksumsMatch(containerData, CONF); + assertNotNull(containerData.getContainerFileChecksum()); assertNotEquals(containerData.ZERO_CHECKSUM, container.getContainerData().getContainerFileChecksum());