From 78da63bac5cfec40b4dcad38d3d4c8998859a91f Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Wed, 19 Feb 2025 09:52:54 -0700 Subject: [PATCH 01/18] atomically import a container --- .../container/keyvalue/KeyValueContainer.java | 8 ++- .../keyvalue/helpers/BlockUtils.java | 2 +- .../DatanodeStoreSchemaThreeImpl.java | 51 +++++++++++++++---- 3 files changed, 48 insertions(+), 13 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java index c5af0c7d9ed2..c918a707414e 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.container.keyvalue; +import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.RECOVERING; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.CONTAINER_ALREADY_EXISTS; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.CONTAINER_FILES_CREATE_ERROR; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.CONTAINER_INTERNAL_ERROR; @@ -666,7 +667,8 @@ public void importContainerData(InputStream input, public void importContainerData(KeyValueContainerData originalContainerData) throws IOException { - containerData.setState(originalContainerData.getState()); + // place the container in the Recovering state while it is being imported + containerData.setState(RECOVERING); containerData .setContainerDBType(originalContainerData.getContainerDBType()); containerData.setSchemaVersion(originalContainerData.getSchemaVersion()); @@ -681,6 +683,10 @@ public void importContainerData(KeyValueContainerData originalContainerData) //fill in memory stat counter (keycount, byte usage) KeyValueContainerUtil.parseKVContainerData(containerData, config); + + // restore imported container's state to the original state and flush the yaml file + containerData.setState(originalContainerData.getState()); + update(originalContainerData.getMetadata(), true); } @Override diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java index 730689539f94..34af206fa0de 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java @@ -328,7 +328,7 @@ public static void loadKVContainerDataFromFiles( File metaDir = new File(containerData.getMetadataPath()); File dumpDir = DatanodeStoreSchemaThreeImpl.getDumpDir(metaDir); try { - store.loadKVContainerData(dumpDir); + store.loadKVContainerData(dumpDir, store); } catch (IOException e) { // Don't delete unloaded or partially loaded files on failure, // but delete all partially loaded metadata. diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java index 9a4b8d4adbf1..4ecaf9cd365a 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java @@ -31,17 +31,25 @@ import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature; import org.apache.hadoop.hdds.utils.MetadataKeyFilters; import org.apache.hadoop.hdds.utils.db.BatchOperation; +import org.apache.hadoop.hdds.utils.db.Codec; import org.apache.hadoop.hdds.utils.db.FixedLengthStringCodec; +import org.apache.hadoop.hdds.utils.db.LongCodec; +import org.apache.hadoop.hdds.utils.db.Proto2Codec; import org.apache.hadoop.hdds.utils.db.RDBStore; import org.apache.hadoop.hdds.utils.db.RocksDatabase; import org.apache.hadoop.hdds.utils.db.RocksDatabase.ColumnFamily; import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.hdds.utils.db.managed.ManagedCompactRangeOptions; +import org.apache.hadoop.hdds.utils.db.managed.ManagedOptions; +import org.apache.hadoop.hdds.utils.db.managed.ManagedReadOptions; +import org.apache.hadoop.hdds.utils.db.managed.ManagedSstFileReader; +import org.apache.hadoop.hdds.utils.db.managed.ManagedSstFileReaderIterator; import org.apache.hadoop.ozone.container.common.helpers.BlockData; import org.apache.hadoop.ozone.container.common.interfaces.BlockIterator; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; import org.apache.hadoop.ozone.container.upgrade.VersionedDatanodeFeatures; import org.rocksdb.LiveFileMetaData; +import org.rocksdb.RocksDBException; /** * Constructs a datanode store in accordance with schema version 3, which uses @@ -131,18 +139,39 @@ public void dumpKVContainerData(long containerID, File dumpDir) prefix); } - public void loadKVContainerData(File dumpDir) - throws IOException { - getMetadataTable().loadFromFile( - getTableDumpFile(getMetadataTable(), dumpDir)); - getBlockDataTable().loadFromFile( - getTableDumpFile(getBlockDataTable(), dumpDir)); - if (VersionedDatanodeFeatures.isFinalized(HDDSLayoutFeature.HBASE_SUPPORT)) { - getLastChunkInfoTable().loadFromFile( - getTableDumpFile(getLastChunkInfoTable(), dumpDir)); + public void loadKVContainerData(File dumpDir, DatanodeStoreSchemaThreeImpl store) throws IOException { + try (BatchOperation batch = store.getBatchHandler().initBatchOperation()) { + processTable(batch, getTableDumpFile(getMetadataTable(), dumpDir), + FixedLengthStringCodec.get(), LongCodec.get(), getMetadataTable()); + processTable(batch, getTableDumpFile(getBlockDataTable(), dumpDir), + FixedLengthStringCodec.get(), BlockData.getCodec(), getBlockDataTable()); + if (VersionedDatanodeFeatures.isFinalized(HDDSLayoutFeature.HBASE_SUPPORT)) { + processTable(batch, getTableDumpFile(getLastChunkInfoTable(), dumpDir), + FixedLengthStringCodec.get(), BlockData.getCodec(), getLastChunkInfoTable()); + } + processTable(batch, getTableDumpFile(getDeleteTransactionTable(), dumpDir), FixedLengthStringCodec.get(), + Proto2Codec.get(DeletedBlocksTransaction.getDefaultInstance()), getDeleteTransactionTable()); + + store.getStore().commitBatchOperation(batch); + } + } + + private void processTable(BatchOperation batch, File tableDumpFile, + Codec keyCodec, Codec valueCodec, Table table) throws IOException { + try (ManagedSstFileReader sstFileReader = new ManagedSstFileReader(new ManagedOptions()); + ManagedSstFileReaderIterator iterator = + ManagedSstFileReaderIterator.managed(sstFileReader.newIterator(new ManagedReadOptions()))) { + sstFileReader.open(tableDumpFile.getAbsolutePath()); + for (iterator.get().seekToFirst(); iterator.get().isValid(); iterator.get().next()) { + byte[] key = iterator.get().key(); + byte[] value = iterator.get().value(); + K decodedKey = keyCodec.fromPersistedFormat(key); + V decodedValue = valueCodec.fromPersistedFormat(value); + table.putWithBatch(batch, decodedKey, decodedValue); + } + } catch (RocksDBException e) { + LOG.error("Failed to import SST file", e); } - getDeleteTransactionTable().loadFromFile( - getTableDumpFile(getDeleteTransactionTable(), dumpDir)); } public static File getTableDumpFile(Table table, From 9ccda664c90488c0003030444505955b50503829 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Thu, 20 Feb 2025 08:18:06 -0700 Subject: [PATCH 02/18] HDDS-12233. throw the RocksDBException --- .../ozone/container/keyvalue/helpers/BlockUtils.java | 3 ++- .../container/metadata/DatanodeStoreSchemaThreeImpl.java | 7 +++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java index 34af206fa0de..ed846a663991 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java @@ -47,6 +47,7 @@ import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaOneImpl; import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaThreeImpl; import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaTwoImpl; +import org.rocksdb.RocksDBException; /** * Utils functions to help block functions. @@ -329,7 +330,7 @@ public static void loadKVContainerDataFromFiles( File dumpDir = DatanodeStoreSchemaThreeImpl.getDumpDir(metaDir); try { store.loadKVContainerData(dumpDir, store); - } catch (IOException e) { + } catch (IOException | RocksDBException e) { // Don't delete unloaded or partially loaded files on failure, // but delete all partially loaded metadata. store.removeKVContainerData(containerID); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java index 4ecaf9cd365a..765adb65fc4c 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java @@ -139,7 +139,8 @@ public void dumpKVContainerData(long containerID, File dumpDir) prefix); } - public void loadKVContainerData(File dumpDir, DatanodeStoreSchemaThreeImpl store) throws IOException { + public void loadKVContainerData(File dumpDir, DatanodeStoreSchemaThreeImpl store) + throws IOException, RocksDBException { try (BatchOperation batch = store.getBatchHandler().initBatchOperation()) { processTable(batch, getTableDumpFile(getMetadataTable(), dumpDir), FixedLengthStringCodec.get(), LongCodec.get(), getMetadataTable()); @@ -157,7 +158,7 @@ public void loadKVContainerData(File dumpDir, DatanodeStoreSchemaThreeImpl store } private void processTable(BatchOperation batch, File tableDumpFile, - Codec keyCodec, Codec valueCodec, Table table) throws IOException { + Codec keyCodec, Codec valueCodec, Table table) throws IOException, RocksDBException { try (ManagedSstFileReader sstFileReader = new ManagedSstFileReader(new ManagedOptions()); ManagedSstFileReaderIterator iterator = ManagedSstFileReaderIterator.managed(sstFileReader.newIterator(new ManagedReadOptions()))) { @@ -169,8 +170,6 @@ private void processTable(BatchOperation batch, File tableDumpFile, V decodedValue = valueCodec.fromPersistedFormat(value); table.putWithBatch(batch, decodedKey, decodedValue); } - } catch (RocksDBException e) { - LOG.error("Failed to import SST file", e); } } From 920e5ad5f8ea02e8979ead3640c7deedf257093b Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Thu, 20 Feb 2025 19:19:08 -0700 Subject: [PATCH 03/18] HDDS-12233. save custom container state --- .../container/keyvalue/TarContainerPacker.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) 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 5d3c001eaf73..470f6bd058c7 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 @@ -22,6 +22,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.SCHEMA_V3; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import java.io.BufferedOutputStream; import java.io.ByteArrayOutputStream; import java.io.File; @@ -42,8 +43,10 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.hadoop.hdds.HddsUtils; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException; import org.apache.hadoop.ozone.OzoneConsts; +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; import org.apache.hadoop.ozone.container.keyvalue.helpers.KeyValueContainerLocationUtil; @@ -90,6 +93,7 @@ public byte[] unpackContainerData(Container container, Path dbRoot = getDbPath(containerUntarDir, containerData); Path chunksRoot = getChunkPath(containerUntarDir, containerData); byte[] descriptorFileContent = innerUnpack(input, dbRoot, chunksRoot); + persistCustomContainerState(container, descriptorFileContent, ContainerProtos.ContainerDataProto.State.RECOVERING); if (!Files.exists(destContainerDir)) { Files.createDirectories(destContainerDir); @@ -108,6 +112,17 @@ public byte[] unpackContainerData(Container container, return descriptorFileContent; } + private void persistCustomContainerState(Container container, + byte[] descriptorContent, ContainerProtos.ContainerDataProto.State state) throws IOException { + Preconditions.checkNotNull(descriptorContent, + "Container descriptor is missing for container {}" + container.getContainerData().getContainerID()); + + KeyValueContainerData originalContainerData = + (KeyValueContainerData) ContainerDataYaml.readContainer(descriptorContent); + originalContainerData.setState(state); + container.update(originalContainerData.getMetadata(), true); + } + private void extractEntry(ArchiveEntry entry, InputStream input, long size, Path ancestor, Path path) throws IOException { HddsUtils.validatePath(path, ancestor); From c9f1741be49027ad2945cbe7950244106ab8b5b5 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 24 Feb 2025 08:29:40 -0800 Subject: [PATCH 04/18] persist state before atomic move, add rocksdb checks, fix tests --- .../keyvalue/TarContainerPacker.java | 28 ++++++++++--- .../DatanodeStoreSchemaThreeImpl.java | 42 +++++++++++++++---- .../keyvalue/TestKeyValueContainer.java | 1 + .../keyvalue/TestTarContainerPacker.java | 24 ++++++++--- 4 files changed, 75 insertions(+), 20 deletions(-) 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 470f6bd058c7..688bc6b4cd02 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 @@ -22,7 +22,6 @@ import static org.apache.hadoop.ozone.OzoneConsts.SCHEMA_V3; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Preconditions; import java.io.BufferedOutputStream; import java.io.ByteArrayOutputStream; import java.io.File; @@ -44,6 +43,7 @@ import org.apache.commons.io.IOUtils; import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; +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.common.impl.ContainerDataYaml; @@ -52,12 +52,15 @@ import org.apache.hadoop.ozone.container.keyvalue.helpers.KeyValueContainerLocationUtil; import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaThreeImpl; import org.apache.hadoop.ozone.container.replication.CopyContainerCompression; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Compress/uncompress KeyValueContainer data to a tar archive. */ public class TarContainerPacker implements ContainerPacker { + private static final Logger LOG = LoggerFactory.getLogger(TarContainerPacker.class); static final String CHUNKS_DIR_NAME = OzoneConsts.STORAGE_DIR_CHUNKS; @@ -99,9 +102,19 @@ public byte[] unpackContainerData(Container container, Files.createDirectories(destContainerDir); } if (FileUtils.isEmptyDirectory(destContainerDir.toFile())) { + // 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 + Path containerMetadataPath = Paths.get(container.getContainerData().getMetadataPath()); + Path tempContainerMetadataPath = Paths.get(containerUntarDir.toString(), + containerMetadataPath.getName(containerMetadataPath.getNameCount() - 1).toString()); + container.getContainerData().setMetadataPath(tempContainerMetadataPath.toString()); + persistCustomContainerState(container, descriptorFileContent, State.RECOVERING); + container.getContainerData().setMetadataPath(containerMetadataPath.toString()); Files.move(containerUntarDir, destContainerDir, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + // Persist again to update the metadata path to point the destination dir + persistCustomContainerState(container, descriptorFileContent, State.RECOVERING); } else { String errorMessage = "Container " + containerId + " unpack failed because ContainerFile " + @@ -112,14 +125,17 @@ public byte[] unpackContainerData(Container container, return descriptorFileContent; } - private void persistCustomContainerState(Container container, - byte[] descriptorContent, ContainerProtos.ContainerDataProto.State state) throws IOException { - Preconditions.checkNotNull(descriptorContent, - "Container descriptor is missing for container {}" + container.getContainerData().getContainerID()); + private void persistCustomContainerState(Container container, byte[] descriptorContent, + ContainerProtos.ContainerDataProto.State state) throws IOException { + if (descriptorContent == null) { + LOG.warn("Skipping persisting of custom state. Container descriptor is null for container {}", + container.getContainerData().getContainerID()); + return; + } KeyValueContainerData originalContainerData = (KeyValueContainerData) ContainerDataYaml.readContainer(descriptorContent); - originalContainerData.setState(state); + container.getContainerData().setState(state); container.update(originalContainerData.getMetadata(), true); } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java index 765adb65fc4c..2fa60f259d46 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java @@ -159,18 +159,42 @@ public void loadKVContainerData(File dumpDir, DatanodeStoreSchemaThreeImpl store private void processTable(BatchOperation batch, File tableDumpFile, Codec keyCodec, Codec valueCodec, Table table) throws IOException, RocksDBException { - try (ManagedSstFileReader sstFileReader = new ManagedSstFileReader(new ManagedOptions()); - ManagedSstFileReaderIterator iterator = - ManagedSstFileReaderIterator.managed(sstFileReader.newIterator(new ManagedReadOptions()))) { + if (isFileEmpty(tableDumpFile)) { + LOG.warn("SST File {} is empty. Skipping processing.", tableDumpFile.getAbsolutePath()); + return; + } + + try (ManagedOptions managedOptions = new ManagedOptions(); + ManagedSstFileReader sstFileReader = new ManagedSstFileReader(managedOptions)) { sstFileReader.open(tableDumpFile.getAbsolutePath()); - for (iterator.get().seekToFirst(); iterator.get().isValid(); iterator.get().next()) { - byte[] key = iterator.get().key(); - byte[] value = iterator.get().value(); - K decodedKey = keyCodec.fromPersistedFormat(key); - V decodedValue = valueCodec.fromPersistedFormat(value); - table.putWithBatch(batch, decodedKey, decodedValue); + try (ManagedReadOptions managedReadOptions = new ManagedReadOptions(); + ManagedSstFileReaderIterator iterator = + ManagedSstFileReaderIterator.managed(sstFileReader.newIterator(managedReadOptions))) { + for (iterator.get().seekToFirst(); iterator.get().isValid(); iterator.get().next()) { + byte[] key = iterator.get().key(); + byte[] value = iterator.get().value(); + K decodedKey = keyCodec.fromPersistedFormat(key); + V decodedValue = valueCodec.fromPersistedFormat(value); + table.putWithBatch(batch, decodedKey, decodedValue); + } + LOG.info("ATTENTION! Finished processing SST file: {}", tableDumpFile.getAbsolutePath()); + } catch (Exception e) { + LOG.error("ATTENTION! Error while processing table from SST file with ManagedSstFileReaderIterator {}: {}", + tableDumpFile.getAbsolutePath(), e.getMessage(), e); + throw e; } + } catch (Exception e) { + LOG.error("ATTENTION! Error while processing table from SST file with ManagedSstFileReader {}: {}", + tableDumpFile.getAbsolutePath(), e.getMessage(), e); + throw e; + } + } + + boolean isFileEmpty(File file) { + if (!file.exists()) { + return true; } + return file.length() == 0; } public static File getTableDumpFile(Table table, 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 083afa4b0560..ed3277e17a07 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 @@ -883,6 +883,7 @@ void testAutoCompactionSmallSstFile( TarContainerPacker packer = new TarContainerPacker(NO_COMPRESSION); container.importContainerData(fis, packer); containerList.add(container); + assertEquals(ContainerProtos.ContainerDataProto.State.CLOSED, container.getContainerData().getState()); } } 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 62d479175553..5b098496210e 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 @@ -72,7 +72,20 @@ public class TestTarContainerPacker { private static final String TEST_CHUNK_FILE_CONTENT = "This is a chunk"; - private static final String TEST_DESCRIPTOR_FILE_CONTENT = "descriptor"; + private static final String TEST_DESCRIPTOR_FILE_CONTENT = "!\n" + + "checksum: 2215d39f2ae1de89fec837d18dc6387d8cba22fb5943cf4616f80c4b34e2edfe\n" + + "chunksPath: target/test-dir/MiniOzoneClusterImpl-23c1bb30-d86a-4f79-88dc-574d8259a5b3/ozone-meta/datanode-4/data-0/hdds/23c1bb30-d86a-4f79-88dc-574d8259a5b3/current/containerDir0/1/chunks\n" + + "containerDBType: RocksDB\n" + + "containerID: 1\n" + + "containerType: KeyValueContainer\n" + + "layOutVersion: 2\n" + + "maxSize: 5368709120\n" + + "metadata: {}\n" + + "metadataPath: target/test-dir/MiniOzoneClusterImpl-23c1bb30-d86a-4f79-88dc-574d8259a5b3/ozone-meta/datanode-4/data-0/hdds/23c1bb30-d86a-4f79-88dc-574d8259a5b3/current/containerDir0/1/metadata\n" + + "originNodeId: 25a48afa-f8d8-44ff-b268-642167e5354b\n" + + "originPipelineId: d7faca81-407f-4a50-a399-bd478c9795e5\n" + + "schemaVersion: '3'\n" + + "state: CLOSED"; private TarContainerPacker packer; @@ -142,9 +155,9 @@ private KeyValueContainerData createContainer(Path dir, boolean createDir) long id = CONTAINER_ID.getAndIncrement(); Path containerDir = dir.resolve(String.valueOf(id)); - Path dbDir = containerDir.resolve("db"); Path dataDir = containerDir.resolve("chunks"); Path metaDir = containerDir.resolve("metadata"); + Path dbDir = metaDir.resolve("db"); if (createDir) { Files.createDirectories(metaDir); Files.createDirectories(dbDir); @@ -245,9 +258,10 @@ public void pack(ContainerTestVersionInfo versionInfo, assertExampleChunkFileIsGood( Paths.get(destinationContainerData.getChunksPath()), TEST_CHUNK_FILE_NAME); - assertFalse(destinationContainer.getContainerFile().exists(), - "Descriptor file should not have been extracted by the " - + "unpackContainerData Call"); + + String containerFileData = new String(Files.readAllBytes(destinationContainer.getContainerFile().toPath()), UTF_8); + assertTrue(containerFileData.contains("RECOVERING"), + "The state of the container is not 'RECOVERING' in the container file"); assertEquals(TEST_DESCRIPTOR_FILE_CONTENT, descriptor); inputForUnpackData.assertClosedExactlyOnce(); } From b3abc3786a5740cbcf2e1e68eb00785b83576d91 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 24 Feb 2025 08:57:38 -0800 Subject: [PATCH 05/18] persist state before atomic move --- .../hadoop/ozone/container/keyvalue/TarContainerPacker.java | 1 - 1 file changed, 1 deletion(-) 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 688bc6b4cd02..894fe9f2f697 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 @@ -96,7 +96,6 @@ public byte[] unpackContainerData(Container container, Path dbRoot = getDbPath(containerUntarDir, containerData); Path chunksRoot = getChunkPath(containerUntarDir, containerData); byte[] descriptorFileContent = innerUnpack(input, dbRoot, chunksRoot); - persistCustomContainerState(container, descriptorFileContent, ContainerProtos.ContainerDataProto.State.RECOVERING); if (!Files.exists(destContainerDir)) { Files.createDirectories(destContainerDir); From a151bd287437fef0cf643e2629699c09f19c1273 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 24 Feb 2025 10:17:53 -0800 Subject: [PATCH 06/18] fix KeyValueContainer test --- .../ozone/container/keyvalue/TestKeyValueContainer.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 ed3277e17a07..693c3545dc90 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 @@ -893,12 +893,16 @@ void testAutoCompactionSmallSstFile( CONF).getStore(); List fileMetaDataList1 = ((RDBStore)(dnStore.getStore())).getDb().getLiveFilesMetaData(); + // When using Table.loadFromFile() in loadKVContainerData(), + // there were as many SST files generated as the number of imported containers + // After moving away from using Table.loadFromFile(), no SST files are generated unless the db is force flushed + assertEquals(0, fileMetaDataList1.size()); hddsVolume.compactDb(); // Sleep a while to wait for compaction to complete Thread.sleep(7000); List fileMetaDataList2 = ((RDBStore)(dnStore.getStore())).getDb().getLiveFilesMetaData(); - assertThat(fileMetaDataList2.size()).isLessThan(fileMetaDataList1.size()); + assertThat(fileMetaDataList2).hasSizeLessThanOrEqualTo(fileMetaDataList1.size()); } finally { // clean up for (KeyValueContainer c : containerList) { From e68c85eab9960a9ba6e392819ebaf2707153757a Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 24 Feb 2025 10:31:50 -0800 Subject: [PATCH 07/18] remove log lines --- .../container/keyvalue/helpers/BlockUtils.java | 2 +- .../metadata/DatanodeStoreSchemaThreeImpl.java | 15 +++------------ 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java index ed846a663991..69f04dc21aaa 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java @@ -329,7 +329,7 @@ public static void loadKVContainerDataFromFiles( File metaDir = new File(containerData.getMetadataPath()); File dumpDir = DatanodeStoreSchemaThreeImpl.getDumpDir(metaDir); try { - store.loadKVContainerData(dumpDir, store); + store.loadKVContainerData(dumpDir); } catch (IOException | RocksDBException e) { // Don't delete unloaded or partially loaded files on failure, // but delete all partially loaded metadata. diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java index 2fa60f259d46..15c1d8800648 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java @@ -139,9 +139,9 @@ public void dumpKVContainerData(long containerID, File dumpDir) prefix); } - public void loadKVContainerData(File dumpDir, DatanodeStoreSchemaThreeImpl store) + public void loadKVContainerData(File dumpDir) throws IOException, RocksDBException { - try (BatchOperation batch = store.getBatchHandler().initBatchOperation()) { + try (BatchOperation batch = getBatchHandler().initBatchOperation()) { processTable(batch, getTableDumpFile(getMetadataTable(), dumpDir), FixedLengthStringCodec.get(), LongCodec.get(), getMetadataTable()); processTable(batch, getTableDumpFile(getBlockDataTable(), dumpDir), @@ -153,7 +153,7 @@ public void loadKVContainerData(File dumpDir, DatanodeStoreSchemaThreeImpl store processTable(batch, getTableDumpFile(getDeleteTransactionTable(), dumpDir), FixedLengthStringCodec.get(), Proto2Codec.get(DeletedBlocksTransaction.getDefaultInstance()), getDeleteTransactionTable()); - store.getStore().commitBatchOperation(batch); + getStore().commitBatchOperation(batch); } } @@ -177,16 +177,7 @@ private void processTable(BatchOperation batch, File tableDumpFile, V decodedValue = valueCodec.fromPersistedFormat(value); table.putWithBatch(batch, decodedKey, decodedValue); } - LOG.info("ATTENTION! Finished processing SST file: {}", tableDumpFile.getAbsolutePath()); - } catch (Exception e) { - LOG.error("ATTENTION! Error while processing table from SST file with ManagedSstFileReaderIterator {}: {}", - tableDumpFile.getAbsolutePath(), e.getMessage(), e); - throw e; } - } catch (Exception e) { - LOG.error("ATTENTION! Error while processing table from SST file with ManagedSstFileReader {}: {}", - tableDumpFile.getAbsolutePath(), e.getMessage(), e); - throw e; } } From 6d9b112ae52a1765b576483b2786f52627c378ce Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 24 Feb 2025 10:50:38 -0800 Subject: [PATCH 08/18] add test to confirm the state of the imported container --- .../container/TestContainerReplication.java | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReplication.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReplication.java index 7c70281315f5..0a071242e998 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReplication.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReplication.java @@ -26,12 +26,14 @@ import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CONTAINER_PLACEMENT_IMPL_KEY; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_DEADNODE_INTERVAL; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_STALENODE_INTERVAL; +import static org.apache.hadoop.ozone.container.TestHelper.isContainerClosed; import static org.apache.hadoop.ozone.container.TestHelper.waitForContainerClose; import static org.apache.hadoop.ozone.container.TestHelper.waitForReplicaCount; import static org.apache.ozone.test.GenericTestUtils.setLogLevel; 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; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.Mockito.any; @@ -284,6 +286,43 @@ private static void deleteContainer(MiniOzoneCluster cluster, DatanodeDetails dn } + @Test + public void testImportedContainerIsClosed() throws Exception { + OzoneConfiguration conf = createConfiguration(false); + // create a 4 node cluster + try (MiniOzoneCluster cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(4).build()) { + cluster.waitForClusterToBeReady(); + + try (OzoneClient client = OzoneClientFactory.getRpcClient(conf)) { + List allNodes = + cluster.getHddsDatanodes().stream() + .map(HddsDatanodeService::getDatanodeDetails) + .collect(Collectors.toList()); + // shutdown 4th node (node 3 is down now) + cluster.shutdownHddsDatanode(allNodes.get(allNodes.size() - 1)); + + createTestData(client); + final OmKeyLocationInfo keyLocation = lookupKeyFirstLocation(cluster); + long containerID = keyLocation.getContainerID(); + waitForContainerClose(cluster, containerID); + + // shutdown nodes 0 and 1. only node 2 is up now + for (int i = 0; i < 2; i++) { + cluster.shutdownHddsDatanode(allNodes.get(i)); + } + waitForReplicaCount(containerID, 1, cluster); + + // bring back up the 4th node + cluster.restartHddsDatanode(allNodes.get(allNodes.size() - 1), false); + + // the container should have been imported on the 4th node + waitForReplicaCount(containerID,2, cluster); + assertTrue(isContainerClosed(cluster, containerID, allNodes.get(allNodes.size() - 1))); + } + } + } + + @Test @Flaky("HDDS-11087") public void testECContainerReplication() throws Exception { From c9907f8f3d9a5ba8e151d165a8a69b15166c575d Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 24 Feb 2025 11:19:27 -0800 Subject: [PATCH 09/18] fix checkstyle error --- .../ozone/container/keyvalue/TestTarContainerPacker.java | 7 ++++--- 1 file changed, 4 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 5b098496210e..e64b00e12542 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 @@ -23,7 +23,6 @@ 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; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -74,14 +73,16 @@ public class TestTarContainerPacker { private static final String TEST_DESCRIPTOR_FILE_CONTENT = "!\n" + "checksum: 2215d39f2ae1de89fec837d18dc6387d8cba22fb5943cf4616f80c4b34e2edfe\n" + - "chunksPath: target/test-dir/MiniOzoneClusterImpl-23c1bb30-d86a-4f79-88dc-574d8259a5b3/ozone-meta/datanode-4/data-0/hdds/23c1bb30-d86a-4f79-88dc-574d8259a5b3/current/containerDir0/1/chunks\n" + + "chunksPath: target/test-dir/MiniOzoneClusterImpl-23c1bb30-d86a-4f79-88dc-574d8259a5b3/ozone-meta/datanode-4" + + "/data-0/hdds/23c1bb30-d86a-4f79-88dc-574d8259a5b3/current/containerDir0/1/chunks\n" + "containerDBType: RocksDB\n" + "containerID: 1\n" + "containerType: KeyValueContainer\n" + "layOutVersion: 2\n" + "maxSize: 5368709120\n" + "metadata: {}\n" + - "metadataPath: target/test-dir/MiniOzoneClusterImpl-23c1bb30-d86a-4f79-88dc-574d8259a5b3/ozone-meta/datanode-4/data-0/hdds/23c1bb30-d86a-4f79-88dc-574d8259a5b3/current/containerDir0/1/metadata\n" + + "metadataPath: target/test-dir/MiniOzoneClusterImpl-23c1bb30-d86a-4f79-88dc-574d8259a5b3/ozone-meta/datanode-4" + + "/data-0/hdds/23c1bb30-d86a-4f79-88dc-574d8259a5b3/current/containerDir0/1/metadata\n" + "originNodeId: 25a48afa-f8d8-44ff-b268-642167e5354b\n" + "originPipelineId: d7faca81-407f-4a50-a399-bd478c9795e5\n" + "schemaVersion: '3'\n" + From 8d09dcb1107e6d2286fb3166c1a91c2277943d94 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 24 Feb 2025 11:20:16 -0800 Subject: [PATCH 10/18] fix checkstyle error --- .../apache/hadoop/ozone/container/TestContainerReplication.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReplication.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReplication.java index 0a071242e998..e773ec9071d5 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReplication.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/container/TestContainerReplication.java @@ -316,7 +316,7 @@ public void testImportedContainerIsClosed() throws Exception { cluster.restartHddsDatanode(allNodes.get(allNodes.size() - 1), false); // the container should have been imported on the 4th node - waitForReplicaCount(containerID,2, cluster); + waitForReplicaCount(containerID, 2, cluster); assertTrue(isContainerClosed(cluster, containerID, allNodes.get(allNodes.size() - 1))); } } From 8e3f9413354c3dffd5099ddea38701f67e5662d1 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Tue, 25 Feb 2025 02:18:51 -0800 Subject: [PATCH 11/18] fix checkstyle error --- .../hadoop/ozone/container/keyvalue/TestTarContainerPacker.java | 1 - 1 file changed, 1 deletion(-) 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 3e7ac65470a0..f3870a136b39 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 @@ -23,7 +23,6 @@ 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; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; From 06e90e287aaa253c48933f114b13742037029d11 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 3 Mar 2025 20:27:02 -0800 Subject: [PATCH 12/18] fix review comments --- .../DatanodeStoreSchemaThreeImpl.java | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java index 10b311674b6e..1dc05ad443dc 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java @@ -33,8 +33,6 @@ import org.apache.hadoop.hdds.utils.db.BatchOperation; import org.apache.hadoop.hdds.utils.db.Codec; import org.apache.hadoop.hdds.utils.db.FixedLengthStringCodec; -import org.apache.hadoop.hdds.utils.db.LongCodec; -import org.apache.hadoop.hdds.utils.db.Proto2Codec; import org.apache.hadoop.hdds.utils.db.RDBStore; import org.apache.hadoop.hdds.utils.db.RocksDatabase; import org.apache.hadoop.hdds.utils.db.RocksDatabase.ColumnFamily; @@ -141,17 +139,26 @@ public void dumpKVContainerData(long containerID, File dumpDir) public void loadKVContainerData(File dumpDir) throws IOException, RocksDBException { + try (BatchOperation batch = getBatchHandler().initBatchOperation()) { processTable(batch, getTableDumpFile(getMetadataTable(), dumpDir), - FixedLengthStringCodec.get(), LongCodec.get(), getMetadataTable()); + getDbDef().getMetadataColumnFamily().getKeyCodec(), + getDbDef().getMetadataColumnFamily().getValueCodec(), + getMetadataTable()); processTable(batch, getTableDumpFile(getBlockDataTable(), dumpDir), - FixedLengthStringCodec.get(), BlockData.getCodec(), getBlockDataTable()); + getDbDef().getBlockDataColumnFamily().getKeyCodec(), + getDbDef().getBlockDataColumnFamily().getValueCodec(), + getBlockDataTable()); if (VersionedDatanodeFeatures.isFinalized(HDDSLayoutFeature.HBASE_SUPPORT)) { processTable(batch, getTableDumpFile(getLastChunkInfoTable(), dumpDir), - FixedLengthStringCodec.get(), BlockData.getCodec(), getLastChunkInfoTable()); + getDbDef().getLastChunkInfoColumnFamily().getKeyCodec(), + getDbDef().getLastChunkInfoColumnFamily().getValueCodec(), + getLastChunkInfoTable()); } - processTable(batch, getTableDumpFile(getDeleteTransactionTable(), dumpDir), FixedLengthStringCodec.get(), - Proto2Codec.get(DeletedBlocksTransaction.getDefaultInstance()), getDeleteTransactionTable()); + processTable(batch, getTableDumpFile(getDeleteTransactionTable(), dumpDir), + ((DatanodeSchemaThreeDBDefinition)getDbDef()).getDeleteTransactionsColumnFamily().getKeyCodec(), + ((DatanodeSchemaThreeDBDefinition)getDbDef()).getDeleteTransactionsColumnFamily().getValueCodec(), + getDeleteTransactionTable()); getStore().commitBatchOperation(batch); } @@ -160,7 +167,7 @@ public void loadKVContainerData(File dumpDir) private void processTable(BatchOperation batch, File tableDumpFile, Codec keyCodec, Codec valueCodec, Table table) throws IOException, RocksDBException { if (isFileEmpty(tableDumpFile)) { - LOG.warn("SST File {} is empty. Skipping processing.", tableDumpFile.getAbsolutePath()); + LOG.debug("SST File {} is empty. Skipping processing.", tableDumpFile.getAbsolutePath()); return; } From 436d151d1d1630bf22151d29f20c359efcf48cb6 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 10 Mar 2025 20:56:39 -0700 Subject: [PATCH 13/18] fix review comments to reduce the number of container file updates --- .../common/interfaces/Container.java | 3 +++ .../container/keyvalue/KeyValueContainer.java | 20 +++++++++---------- .../keyvalue/TarContainerPacker.java | 13 +++++------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Container.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Container.java index 19d61a4a1c1a..85f8f3164905 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Container.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Container.java @@ -128,6 +128,9 @@ void create(VolumeSet volumeSet, VolumeChoosingPolicy volumeChoosingPolicy, void update(Map metaData, boolean forceUpdate) throws StorageContainerException; + void update(Map metaData, boolean forceUpdate, String containerMetadataPath) + throws StorageContainerException; + void updateDataScanTimestamp(Instant timestamp) throws StorageContainerException; diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java index c918a707414e..0889ff8d2039 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java @@ -553,13 +553,17 @@ public ContainerType getContainerType() { } @Override - public void update( - Map metadata, boolean forceUpdate) + public void update(Map metadata, boolean forceUpdate) throws StorageContainerException { + update(metadata, forceUpdate, containerData.getMetadataPath()); + } + @Override + public void update(Map metadata, boolean forceUpdate, String containerMetadataPath) + throws StorageContainerException { // TODO: Now, when writing the updated data to .container file, we are - // holding lock and writing data to disk. We can have async implementation - // to flush the update container data to disk. + // holding lock and writing data to disk. We can have async implementation + // to flush the update container data to disk. long containerId = containerData.getContainerID(); if (!containerData.isValid()) { LOG.debug("Invalid container data. ContainerID: {}", containerId); @@ -579,7 +583,7 @@ public void update( containerData.addMetadata(entry.getKey(), entry.getValue()); } - File containerFile = getContainerFile(); + File containerFile = getContainerFile(containerMetadataPath, containerData.getContainerID()); // update the new container data to .container File updateContainerFile(containerFile); } catch (StorageContainerException ex) { @@ -667,15 +671,10 @@ public void importContainerData(InputStream input, public void importContainerData(KeyValueContainerData originalContainerData) throws IOException { - // place the container in the Recovering state while it is being imported - containerData.setState(RECOVERING); containerData .setContainerDBType(originalContainerData.getContainerDBType()); containerData.setSchemaVersion(originalContainerData.getSchemaVersion()); - //rewriting the yaml file with new checksum calculation. - update(originalContainerData.getMetadata(), true); - if (containerData.hasSchema(OzoneConsts.SCHEMA_V3)) { // load metadata from received dump files before we try to parse kv BlockUtils.loadKVContainerDataFromFiles(containerData, config); @@ -684,6 +683,7 @@ public void importContainerData(KeyValueContainerData originalContainerData) //fill in memory stat counter (keycount, byte usage) KeyValueContainerUtil.parseKVContainerData(containerData, config); + // rewriting the yaml file with new checksum calculation // restore imported container's state to the original state and flush the yaml file containerData.setState(originalContainerData.getState()); update(originalContainerData.getMetadata(), true); 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 894fe9f2f697..c23b52131608 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 @@ -102,18 +102,15 @@ public byte[] unpackContainerData(Container container, } if (FileUtils.isEmptyDirectory(destContainerDir.toFile())) { // 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 + // 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()); - container.getContainerData().setMetadataPath(tempContainerMetadataPath.toString()); - persistCustomContainerState(container, descriptorFileContent, State.RECOVERING); - container.getContainerData().setMetadataPath(containerMetadataPath.toString()); + persistCustomContainerState(container, descriptorFileContent, State.RECOVERING, tempContainerMetadataPath); Files.move(containerUntarDir, destContainerDir, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); - // Persist again to update the metadata path to point the destination dir - persistCustomContainerState(container, descriptorFileContent, State.RECOVERING); } else { String errorMessage = "Container " + containerId + " unpack failed because ContainerFile " + @@ -125,7 +122,7 @@ public byte[] unpackContainerData(Container container, } private void persistCustomContainerState(Container container, byte[] descriptorContent, - ContainerProtos.ContainerDataProto.State state) throws IOException { + ContainerProtos.ContainerDataProto.State state, Path containerMetadataPath) throws IOException { if (descriptorContent == null) { LOG.warn("Skipping persisting of custom state. Container descriptor is null for container {}", container.getContainerData().getContainerID()); @@ -135,7 +132,7 @@ private void persistCustomContainerState(Container contai KeyValueContainerData originalContainerData = (KeyValueContainerData) ContainerDataYaml.readContainer(descriptorContent); container.getContainerData().setState(state); - container.update(originalContainerData.getMetadata(), true); + container.update(originalContainerData.getMetadata(), true, containerMetadataPath.toString()); } private void extractEntry(ArchiveEntry entry, InputStream input, long size, From db1ac272f999230c51d1958e904e51a862f99434 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Mon, 10 Mar 2025 21:13:14 -0700 Subject: [PATCH 14/18] fix review comments to reduce the number of container file updates --- .../hadoop/ozone/container/keyvalue/KeyValueContainer.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java index 0889ff8d2039..9de00987233d 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java @@ -17,7 +17,6 @@ package org.apache.hadoop.ozone.container.keyvalue; -import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State.RECOVERING; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.CONTAINER_ALREADY_EXISTS; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.CONTAINER_FILES_CREATE_ERROR; import static org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.Result.CONTAINER_INTERNAL_ERROR; From 35361d11c591a170993b58056ddcb2f243685cae Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Wed, 19 Mar 2025 12:35:27 -0700 Subject: [PATCH 15/18] delete Ratis replicated recovering containers upon datanode start --- .../ozone/container/ozoneimpl/ContainerReader.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java index 8e4954032b7f..90bbb3186ad4 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/ozoneimpl/ContainerReader.java @@ -211,10 +211,15 @@ public void verifyAndFixupContainerData(ContainerData containerData) config); if (kvContainer.getContainerState() == RECOVERING) { if (shouldDelete) { - kvContainer.markContainerUnhealthy(); - LOG.info("Stale recovering container {} marked UNHEALTHY", - kvContainerData.getContainerID()); - containerSet.addContainer(kvContainer); + // delete Ratis replicated RECOVERING containers + if (kvContainer.getContainerData().getReplicaIndex() == 0) { + cleanupContainer(hddsVolume, kvContainer); + } else { + kvContainer.markContainerUnhealthy(); + LOG.info("Stale recovering container {} marked UNHEALTHY", + kvContainerData.getContainerID()); + containerSet.addContainer(kvContainer); + } } return; } From 1a695005ae745ddac829b7968d47129cb4d20937 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Wed, 19 Mar 2025 14:37:52 -0700 Subject: [PATCH 16/18] fix unit test --- .../container/ozoneimpl/TestContainerReader.java | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) 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 6e488ae2bcc2..edd6487bf9ae 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 @@ -236,10 +236,16 @@ public void testContainerReader(ContainerTestVersionInfo versionInfo) thread.start(); thread.join(); - //recovering container should be marked unhealthy, so the count should be 3 - assertEquals(UNHEALTHY, containerSet.getContainer( - recoveringContainerData.getContainerID()).getContainerState()); - assertEquals(3, containerSet.containerCount()); + // Ratis replicated recovering containers are deleted upon datanode startup + if (recoveringKeyValueContainer.getContainerData().getReplicaIndex() == 0) { + assertNull(containerSet.getContainer(recoveringContainerData.getContainerID())); + assertEquals(2, containerSet.containerCount()); + } else { + //recovering container should be marked unhealthy, so the count should be 3 + assertEquals(UNHEALTHY, containerSet.getContainer( + recoveringContainerData.getContainerID()).getContainerState()); + assertEquals(3, containerSet.containerCount()); + } for (int i = 0; i < 2; i++) { Container keyValueContainer = containerSet.getContainer(i); From f7108cb2cf8bd4b64f503f67038de1f2f07c4cea Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Fri, 28 Mar 2025 10:50:48 -0700 Subject: [PATCH 17/18] HDDS-12233. fix review comments --- .../common/interfaces/ContainerPacker.java | 15 +++++++++++++++ .../container/keyvalue/TarContainerPacker.java | 16 ---------------- .../container/keyvalue/helpers/BlockUtils.java | 3 +-- .../metadata/DatanodeStoreSchemaThreeImpl.java | 4 +++- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerPacker.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerPacker.java index 2d19337db8bf..5d0a0c246586 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerPacker.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerPacker.java @@ -21,7 +21,9 @@ import java.io.InputStream; import java.io.OutputStream; import java.nio.file.Path; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.ozone.container.common.impl.ContainerData; +import org.apache.hadoop.ozone.container.common.impl.ContainerDataYaml; /** * Service to pack/unpack ContainerData container data to/from a single byte @@ -54,4 +56,17 @@ void pack(Container container, OutputStream destination) */ byte[] unpackContainerDescriptor(InputStream inputStream) throws IOException; + + /** + * Persists the custom state for a container. This method allows saving the container file to a custom location. + */ + default void persistCustomContainerState(Container container, byte[] descriptorContent, + ContainerProtos.ContainerDataProto.State state, Path containerMetadataPath) throws IOException { + if (descriptorContent == null) { + return; + } + ContainerData originalContainerData = ContainerDataYaml.readContainer(descriptorContent); + container.getContainerData().setState(state); + container.update(originalContainerData.getMetadata(), true, containerMetadataPath.toString()); + } } 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 c23b52131608..c33e2fb79fb8 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 @@ -42,11 +42,9 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.io.IOUtils; import org.apache.hadoop.hdds.HddsUtils; -import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; 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.common.impl.ContainerDataYaml; 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; @@ -121,20 +119,6 @@ public byte[] unpackContainerData(Container container, return descriptorFileContent; } - private void persistCustomContainerState(Container container, byte[] descriptorContent, - ContainerProtos.ContainerDataProto.State state, Path containerMetadataPath) throws IOException { - if (descriptorContent == null) { - LOG.warn("Skipping persisting of custom state. Container descriptor is null for container {}", - container.getContainerData().getContainerID()); - return; - } - - KeyValueContainerData originalContainerData = - (KeyValueContainerData) ContainerDataYaml.readContainer(descriptorContent); - container.getContainerData().setState(state); - container.update(originalContainerData.getMetadata(), true, containerMetadataPath.toString()); - } - private void extractEntry(ArchiveEntry entry, InputStream input, long size, Path ancestor, Path path) throws IOException { HddsUtils.validatePath(path, ancestor); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java index 69f04dc21aaa..730689539f94 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/BlockUtils.java @@ -47,7 +47,6 @@ import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaOneImpl; import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaThreeImpl; import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaTwoImpl; -import org.rocksdb.RocksDBException; /** * Utils functions to help block functions. @@ -330,7 +329,7 @@ public static void loadKVContainerDataFromFiles( File dumpDir = DatanodeStoreSchemaThreeImpl.getDumpDir(metaDir); try { store.loadKVContainerData(dumpDir); - } catch (IOException | RocksDBException e) { + } catch (IOException e) { // Don't delete unloaded or partially loaded files on failure, // but delete all partially loaded metadata. store.removeKVContainerData(containerID); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java index 1dc05ad443dc..98adc30abd82 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/metadata/DatanodeStoreSchemaThreeImpl.java @@ -138,7 +138,7 @@ public void dumpKVContainerData(long containerID, File dumpDir) } public void loadKVContainerData(File dumpDir) - throws IOException, RocksDBException { + throws IOException { try (BatchOperation batch = getBatchHandler().initBatchOperation()) { processTable(batch, getTableDumpFile(getMetadataTable(), dumpDir), @@ -161,6 +161,8 @@ public void loadKVContainerData(File dumpDir) getDeleteTransactionTable()); getStore().commitBatchOperation(batch); + } catch (RocksDBException e) { + throw new IOException("Failed to load container data from dump file.", e); } } From a3f661bde1258c142009a204808ad22b9a5c4787 Mon Sep 17 00:00:00 2001 From: Rishabh Patel Date: Fri, 28 Mar 2025 11:40:42 -0700 Subject: [PATCH 18/18] HDDS-12233. remove unused field --- .../hadoop/ozone/container/keyvalue/TarContainerPacker.java | 3 --- 1 file changed, 3 deletions(-) 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 0b191a4a9cf8..46a2a94975e0 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,15 +47,12 @@ import org.apache.hadoop.ozone.container.keyvalue.helpers.KeyValueContainerLocationUtil; import org.apache.hadoop.ozone.container.metadata.DatanodeStoreSchemaThreeImpl; import org.apache.hadoop.ozone.container.replication.CopyContainerCompression; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Compress/uncompress KeyValueContainer data to a tar archive. */ public class TarContainerPacker implements ContainerPacker { - private static final Logger LOG = LoggerFactory.getLogger(TarContainerPacker.class); static final String CHUNKS_DIR_NAME = OzoneConsts.STORAGE_DIR_CHUNKS;