Skip to content

HDDS-13023. Container checksum is missing after container import #8459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ChenSammi merged 5 commits into from
Jun 20, 2025
Merged

HDDS-13023. Container checksum is missing after container import #8459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9eb945d
update importContainerData logic to resolve checksum missing
May 14, 2025
2ac4848
verify checksum before state changes to recovering and skipping after…
Jun 4, 2025
a25cde5
fixed TestTarContainerPacker failure
Jun 4, 2025
cf5751c
keep the current parseKVContainerData for most existing code call
Jun 18, 2025
9bb5c5f
refactor alignment
Jun 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...er-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/ContainerData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public abstract class ContainerData {
private transient Optional<Instant> lastDataScanTime = Optional.empty();

public static final Charset CHARSET_ENCODING = StandardCharsets.UTF_8;
private static final String DUMMY_CHECKSUM = new String(new byte[64],
public static final String DUMMY_CHECKSUM = new String(new byte[64],
CHARSET_ENCODING);

// Common Fields need to be stored in .container file.
Expand Down
2 changes: 1 addition & 1 deletion ...r-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -659,7 +659,7 @@ public void importContainerData(KeyValueContainerData originalContainerData)
}

//fill in memory stat counter (keycount, byte usage)
KeyValueContainerUtil.parseKVContainerData(containerData, config);
KeyValueContainerUtil.parseKVContainerData(containerData, config, true);

// rewriting the yaml file with new checksum calculation
// restore imported container's state to the original state and flush the yaml file
Expand Down
15 changes: 15 additions & 0 deletions ...-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/TarContainerPacker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,13 @@
import org.apache.commons.compress.archivers.ArchiveOutputStream;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.io.FileUtils;
import org.apache.hadoop.hdds.conf.ConfigurationSource;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
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.helpers.ContainerUtils;
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;
Expand All @@ -62,6 +66,8 @@ public class TarContainerPacker

private final CopyContainerCompression compression;

private final ConfigurationSource conf = new OzoneConfiguration();

public TarContainerPacker(CopyContainerCompression compression) {
this.compression = compression;
}
Expand Down Expand Up @@ -93,6 +99,15 @@ public byte[] unpackContainerData(Container<KeyValueContainerData> container,
Files.createDirectories(destContainerDir);
}
if (FileUtils.isEmptyDirectory(destContainerDir.toFile())) {

//before state change to RECOVERING, we need to verify the checksum for untarContainerData.
if (descriptorFileContent != null) {
KeyValueContainerData untarContainerData =
(KeyValueContainerData) ContainerDataYaml
.readContainer(descriptorFileContent);
ContainerUtils.verifyChecksum(untarContainerData, conf);
}

// 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.
Expand Down
18 changes: 17 additions & 1 deletion ...c/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/KeyValueContainerUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,17 +196,33 @@ public static boolean noBlocksInContainer(DatanodeStore store,
* Parse KeyValueContainerData and verify checksum. Set block related
* metadata like block commit sequence id, block count, bytes used and
* pending delete block count and delete transaction id.
* This method will verify checksum by default.
* @param kvContainerData
* @param config
* @throws IOException
*/
public static void parseKVContainerData(KeyValueContainerData kvContainerData,
ConfigurationSource config) throws IOException {
parseKVContainerData(kvContainerData, config, false);
}

/**
* @param kvContainerData
* @param config
* @param skipVerifyChecksum checksum verification should be skipped if the state
* has changed to RECOVERING during container import, false otherwise
* @throws IOException
*/
public static void parseKVContainerData(KeyValueContainerData kvContainerData,
ConfigurationSource config, boolean skipVerifyChecksum) throws IOException {

long containerID = kvContainerData.getContainerID();

// Verify Checksum
ContainerUtils.verifyChecksum(kvContainerData, config);
// skip verify checksum if the state has changed to RECOVERING during container import
if (!skipVerifyChecksum) {
ContainerUtils.verifyChecksum(kvContainerData, config);
}

if (kvContainerData.getSchemaVersion() == null) {
// If this container has not specified a schema version, it is in the old
Expand Down
2 changes: 2 additions & 0 deletions ...rvice/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,8 @@ public void testContainerImportExport(ContainerTestVersionInfo versionInfo)
containerData.getMaxSize());
assertEquals(keyValueContainerData.getBytesUsed(),
containerData.getBytesUsed());
assertNotNull(containerData.getChecksum());
assertNotEquals(containerData.DUMMY_CHECKSUM, container.getContainerData().getChecksum());

//Can't overwrite existing container
KeyValueContainer finalContainer = container;
Expand Down
2 changes: 1 addition & 1 deletion ...vice/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestTarContainerPacker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public class TestTarContainerPacker {
private static final String TEST_CHUNK_FILE_CONTENT = "This is a chunk";

private static final String TEST_DESCRIPTOR_FILE_CONTENT = "!<KeyValueContainerData>\n" +
"checksum: 2215d39f2ae1de89fec837d18dc6387d8cba22fb5943cf4616f80c4b34e2edfe\n" +
"checksum: 5e4bea7286f96d88a5b3a745011ff9e4281a5221bfe564413215cd85871dcfd8\n" +
Copy link
Contributor

@ChenSammi ChenSammi Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why update this checksum?

Copy link
Contributor Author

@Gargi-jais11 Gargi-jais11 Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The checksum change was necessary because the point of checksum verification shifted. The checksum embedded in TEST_DESCRIPTOR_FILE_CONTENT must now accurately reflect the content of that string before any state changes occur during the unpackContainerData process. So the new checksum 5e4bea... correctly corresponds to the TEST_DESCRIPTOR_FILE_CONTENT which includes state: CLOSED.

"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" +
Expand Down