This is an automated email from the ASF dual-hosted git repository.
smengcl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 0eb1c67cd6a HDDS-15760. Handle large or invalid OM snapshot local data
YAMLs (#10679)
0eb1c67cd6a is described below
commit 0eb1c67cd6ab0d8bd99cc705170dc646650000b7
Author: Siyao Meng <[email protected]>
AuthorDate: Tue Jul 7 11:28:36 2026 -0700
HDDS-15760. Handle large or invalid OM snapshot local data YAMLs (#10679)
---
.../hadoop/ozone/om/OmSnapshotLocalDataYaml.java | 12 +++-
.../om/snapshot/OmSnapshotLocalDataManager.java | 83 +++++++++++++++++++---
.../ozone/om/TestOmSnapshotLocalDataYaml.java | 36 ++++++++++
.../snapshot/TestOmSnapshotLocalDataManager.java | 49 +++++++++++--
4 files changed, 166 insertions(+), 14 deletions(-)
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java
index ed6b3feda7e..24c7fb3e3e7 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java
@@ -56,6 +56,9 @@ public final class OmSnapshotLocalDataYaml {
public static final Tag SNAPSHOT_VERSION_META_TAG = new Tag("VersionMeta");
public static final Tag SST_FILE_INFO_TAG = new Tag("SstFileInfo");
public static final String YAML_FILE_EXTENSION = ".yaml";
+ // Maximum number of Unicode code points the YAML parser will read
(SnakeYAML code point limit).
+ // For the ASCII snapshot local data YAML this is effectively the maximum
file size in bytes.
+ public static final int SNAPSHOT_LOCAL_DATA_YAML_CODE_POINT_LIMIT = 128 *
1024 * 1024;
private OmSnapshotLocalDataYaml() {
}
@@ -125,7 +128,7 @@ protected NodeTuple representJavaBeanProperty(
*/
private static class SnapshotLocalDataConstructor extends SafeConstructor {
SnapshotLocalDataConstructor() {
- super(new LoaderOptions());
+ super(createLoaderOptions());
//Adding our own specific constructors for tags.
this.yamlConstructors.put(SNAPSHOT_YAML_TAG, new
ConstructSnapshotLocalData());
this.yamlConstructors.put(SNAPSHOT_VERSION_META_TAG, new
ConstructVersionMeta());
@@ -138,6 +141,13 @@ private static class SnapshotLocalDataConstructor extends
SafeConstructor {
this.addTypeDescription(versionMetaDesc);
}
+ private static LoaderOptions createLoaderOptions() {
+ LoaderOptions options = new LoaderOptions();
+ // Snapshot local data is trusted local metadata, but keep a finite
parser bound to avoid unbounded memory use.
+ options.setCodePointLimit(SNAPSHOT_LOCAL_DATA_YAML_CODE_POINT_LIMIT);
+ return options;
+ }
+
private final class ConstructSstFileInfo extends AbstractConstruct {
@Override
public Object construct(Node node) {
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java
index a2203f675d8..994cfec7b37 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java
@@ -288,8 +288,28 @@ private void addMissingSnapshotYamlFiles(
}
void addVersionNodeWithDependents(OmSnapshotLocalData snapshotLocalData)
throws IOException {
+ addVersionNodeWithDependents(snapshotLocalData, null);
+ }
+
+ /**
+ * Adds version nodes for the supplied snapshot local data and any unloaded
previous snapshots it depends on.
+ * The graph must contain the previous snapshot's version node before the
current snapshot's version node can be
+ * added. This method walks the previous-snapshot chain using persisted YAML
metadata and processes the stack in
+ * dependency order.
+ *
+ * @param snapshotLocalData snapshot local data to add to the version graph
+ * @param failedFilePaths when non-null, a previous snapshot YAML that
cannot be loaded or whose snapshotId does not
+ * match its path is skipped instead of thrown, and its path is recorded
here so it is not reloaded during
+ * startup (a path recorded here is not logged again on subsequent
lookups)
+ * @return true if the snapshot local data was added or was already present,
false if skipped due to an unloadable or
+ * mismatched previous snapshot
+ * @throws IOException if a required YAML load fails, or the loaded
snapshotId does not match, when failedFilePaths is
+ * null
+ */
+ private boolean addVersionNodeWithDependents(OmSnapshotLocalData
snapshotLocalData, Set<String> failedFilePaths)
+ throws IOException {
if (versionNodeMap.containsKey(snapshotLocalData.getSnapshotId())) {
- return;
+ return true;
}
Set<UUID> visitedSnapshotIds = new HashSet<>();
Stack<Pair<UUID, SnapshotVersionsMeta>> stack = new Stack<>();
@@ -305,17 +325,52 @@ void addVersionNodeWithDependents(OmSnapshotLocalData
snapshotLocalData) throws
UUID prevSnapId = snapshotVersionsMeta.getPreviousSnapshotId();
if (prevSnapId != null && !versionNodeMap.containsKey(prevSnapId)) {
File previousSnapshotLocalDataFile = new
File(getSnapshotLocalPropertyYamlPath(prevSnapId));
- OmSnapshotLocalData prevSnapshotLocalData =
snapshotLocalDataSerializer.load(previousSnapshotLocalDataFile);
+ OmSnapshotLocalData prevSnapshotLocalData;
+ if (failedFilePaths != null) {
+ Optional<OmSnapshotLocalData> loadedLocalData =
+ tryLoadSnapshotLocalData(previousSnapshotLocalDataFile,
failedFilePaths);
+ if (!loadedLocalData.isPresent()) {
+ // tryLoadSnapshotLocalData recorded this path in
failedFilePaths (logging the underlying failure the
+ // first time it was seen). Skip this snapshot so it is not
added to the version graph.
+ return false;
+ }
+ prevSnapshotLocalData = loadedLocalData.get();
+ } else {
+ prevSnapshotLocalData =
snapshotLocalDataSerializer.load(previousSnapshotLocalDataFile);
+ }
if (!prevSnapId.equals(prevSnapshotLocalData.getSnapshotId())) {
- throw new IOException("SnapshotId mismatch: expected " +
prevSnapId +
+ String mismatch = "Expected SnapshotId " + prevSnapId +
" but found " + prevSnapshotLocalData.getSnapshotId() +
- " in file " + previousSnapshotLocalDataFile.getAbsolutePath());
+ " in file " + previousSnapshotLocalDataFile.getAbsolutePath();
+ if (failedFilePaths != null) {
+
failedFilePaths.add(previousSnapshotLocalDataFile.getAbsolutePath());
+ LOG.error("Skipping snapshot local data for snapshot {} because
previous snapshot local data yaml {} " +
+ "has a mismatched snapshotId: {}", snapId,
previousSnapshotLocalDataFile.getAbsolutePath(), mismatch);
+ return false;
+ }
+ throw new IOException(mismatch);
}
stack.push(Pair.of(prevSnapshotLocalData.getSnapshotId(), new
SnapshotVersionsMeta(prevSnapshotLocalData)));
}
visitedSnapshotIds.add(snapId);
}
}
+ return true;
+ }
+
+ private Optional<OmSnapshotLocalData> tryLoadSnapshotLocalData(File
localDataFile, Set<String> failedFilePaths) {
+ String path = localDataFile.getAbsolutePath();
+ if (failedFilePaths.contains(path)) {
+ return Optional.empty();
+ }
+ try {
+ return Optional.of(snapshotLocalDataSerializer.load(localDataFile));
+ } catch (IOException e) {
+ failedFilePaths.add(path);
+ LOG.error("Skipping snapshot local data file {} because it could not be
loaded. " +
+ "Snapshot defrag and snapshot diff may be unavailable for the
affected snapshot.", path, e);
+ return Optional.empty();
+ }
}
private void incrementOrphanCheckCount(UUID snapshotId) {
@@ -360,16 +415,28 @@ private void init(OzoneConfiguration configuration,
SnapshotChainManager chainMa
throw new IOException("Error while listing yaml files inside directory:
" + snapshotDir.getAbsolutePath());
}
Arrays.sort(localDataFiles, Comparator.comparing(File::getName));
+ Set<String> failedFilePaths = new HashSet<>();
for (File localDataFile : localDataFiles) {
- OmSnapshotLocalData snapshotLocalData =
snapshotLocalDataSerializer.load(localDataFile);
+ Optional<OmSnapshotLocalData> loadedLocalData =
tryLoadSnapshotLocalData(localDataFile, failedFilePaths);
+ if (!loadedLocalData.isPresent()) {
+ continue;
+ }
+ OmSnapshotLocalData snapshotLocalData = loadedLocalData.get();
File file = new
File(getSnapshotLocalPropertyYamlPath(snapshotLocalData.getSnapshotId()));
String expectedPath = file.getAbsolutePath();
String actualPath = localDataFile.getAbsolutePath();
if (!expectedPath.equals(actualPath)) {
- throw new IOException("Unexpected path for local data file with
snapshotId:" + snapshotLocalData.getSnapshotId()
- + " : " + actualPath + ". " + "Expected: " + expectedPath);
+ failedFilePaths.add(actualPath);
+ LOG.error("Skipping snapshot local data file {} because its stored
snapshotId {} does not match its path. " +
+ "Expected path: {}.", actualPath,
snapshotLocalData.getSnapshotId(), expectedPath);
+ continue;
+ }
+ if (!addVersionNodeWithDependents(snapshotLocalData, failedFilePaths)) {
+ // A previous snapshot in the dependency chain could not be loaded, so
this snapshot was not added to the
+ // version graph. Record its path as failed so later snapshots that
depend on it short-circuit in
+ // tryLoadSnapshotLocalData instead of reparsing this YAML.
+ failedFilePaths.add(actualPath);
}
- addVersionNodeWithDependents(snapshotLocalData);
}
for (UUID snapshotId : versionNodeMap.keySet()) {
incrementOrphanCheckCount(snapshotId);
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLocalDataYaml.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLocalDataYaml.java
index e18adae7b40..bfcb4b0b990 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLocalDataYaml.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLocalDataYaml.java
@@ -32,6 +32,7 @@
import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
import java.util.Collections;
import java.util.List;
import java.util.Map;
@@ -240,6 +241,41 @@ public void testEmptyFile() throws IOException {
assertThat(ex).hasMessageContaining("Failed to load file. File is empty.");
}
+ @Test
+ public void testLoadYamlLargerThanDefaultSnakeYamlLimit() throws IOException
{
+ UUID snapshotId = UUID.randomUUID();
+ File yamlFile = new File(testRoot, "large-snapshot.yaml");
+ StringBuilder yaml = new StringBuilder(4 * 1024 * 1024);
+ yaml.append("!<OmSnapshotLocalData>\n")
+ .append("checksum:
\"0000000000000000000000000000000000000000000000000000000000000000\"\n")
+ .append("dbTxSequenceNumber: 10\n")
+ .append("isSSTFiltered: false\n")
+ .append("lastDefragTime: 0\n")
+ .append("needsDefrag: false\n")
+ .append("snapshotId: \"").append(snapshotId).append("\"\n")
+ .append("version: 0\n")
+ .append("versionSstFileInfos:\n")
+ .append(" 0: !<VersionMeta>\n")
+ .append(" previousSnapshotVersion: 0\n")
+ .append(" sstFiles:\n");
+ int sstFileCount = 0;
+ while (yaml.length() <= 3 * 1024 * 1024 + 1024) {
+ yaml.append(" - !<SstFileInfo>\n")
+ .append(" fileName:
file-").append(sstFileCount).append(".sst\n")
+ .append(" startKey:
key-start-").append(sstFileCount).append("-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n")
+ .append(" endKey:
key-end-").append(sstFileCount).append("-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n")
+ .append(" columnFamily: fileTable\n");
+ sstFileCount++;
+ }
+ FileUtils.writeStringToFile(yamlFile, yaml.toString(),
StandardCharsets.UTF_8);
+ assertThat(yamlFile.length()).isGreaterThan(3L * 1024 * 1024);
+
+ OmSnapshotLocalData snapshotData =
omSnapshotLocalDataSerializer.load(yamlFile);
+
+ assertThat(snapshotData.getSnapshotId()).isEqualTo(snapshotId);
+
assertThat(snapshotData.getVersionSstFileInfos().get(0).getSstFiles()).hasSize(sstFileCount);
+ }
+
@Test
public void testChecksum() throws IOException {
UUID snapshotId = UUID.randomUUID();
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotLocalDataManager.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotLocalDataManager.java
index c73304fa122..9aecb7b1f28 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotLocalDataManager.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotLocalDataManager.java
@@ -26,6 +26,7 @@
import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE;
import static
org.apache.hadoop.ozone.om.helpers.SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE;
import static
org.apache.hadoop.ozone.om.helpers.SnapshotInfo.SnapshotStatus.SNAPSHOT_DELETED;
+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;
@@ -46,6 +47,7 @@
import com.google.common.collect.ImmutableSet;
import java.io.File;
import java.io.IOException;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
@@ -986,6 +988,42 @@ public void testInitWithExistingYamlFiles() throws
IOException {
assertEquals(versionMap.keySet(), new HashSet<>(versionIds));
}
+ @Test
+ public void testInitSkipsYamlFilesThatCannotBeLoaded() throws IOException {
+ UUID snapshotId = UUID.fromString("00000000-0000-0000-0000-000000000001");
+ UUID validSnapshotId =
UUID.fromString("00000000-0000-0000-0000-000000000002");
+ UUID previousSnapshotId =
UUID.fromString("ffffffff-ffff-ffff-ffff-ffffffffffff");
+
+ createSnapshotLocalDataFile(snapshotId, previousSnapshotId);
+ createSnapshotLocalDataFile(validSnapshotId, null);
+ Path invalidYamlPath = Paths.get(snapshotsDir.getAbsolutePath(),
+ "db" + OM_SNAPSHOT_SEPARATOR + previousSnapshotId +
YAML_FILE_EXTENSION);
+ Files.write(invalidYamlPath, "not:
[valid".getBytes(StandardCharsets.UTF_8));
+
+ localDataManager = getNewOmSnapshotLocalDataManager();
+
+
assertThat(localDataManager.getVersionNodeMapUnmodifiable()).containsOnlyKeys(validSnapshotId);
+ }
+
+ @Test
+ public void testInitSkipsPreviousSnapshotWithMismatchedSnapshotId() throws
IOException {
+ UUID snapshotId = UUID.fromString("00000000-0000-0000-0000-000000000001");
+ UUID validSnapshotId =
UUID.fromString("00000000-0000-0000-0000-000000000002");
+ UUID previousSnapshotId =
UUID.fromString("ffffffff-ffff-ffff-ffff-ffffffffffff");
+ UUID mismatchedSnapshotId =
UUID.fromString("00000000-0000-0000-0000-000000000003");
+
+ createSnapshotLocalDataFile(snapshotId, previousSnapshotId);
+ createSnapshotLocalDataFile(validSnapshotId, null);
+ // Write a loadable YAML at the previous snapshot's path, but whose stored
snapshotId does not match the path.
+ Path mismatchedYamlPath = Paths.get(snapshotsDir.getAbsolutePath(),
+ "db" + OM_SNAPSHOT_SEPARATOR + previousSnapshotId +
YAML_FILE_EXTENSION);
+ writeLocalDataToFile(createMockLocalData(mismatchedSnapshotId, null),
mismatchedYamlPath);
+
+ localDataManager = getNewOmSnapshotLocalDataManager();
+
+
assertThat(localDataManager.getVersionNodeMapUnmodifiable()).containsOnlyKeys(validSnapshotId);
+ }
+
@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testInitWithMissingYamlFiles(boolean needsUpgrade) throws
IOException {
@@ -1081,16 +1119,17 @@ public void
testCheckOrphanSnapshotVersionsWithStaleSnapshotChain() throws IOExc
}
@Test
- public void testInitWithInvalidPathThrowsException() throws IOException {
+ public void testInitSkipsYamlFileWhosePathDoesNotMatchStoredSnapshotId()
throws IOException {
UUID snapshotId = UUID.randomUUID();
-
+
// Create a file with wrong location
OmSnapshotLocalData localData = createMockLocalData(snapshotId, null);
Path wrongPath = Paths.get(snapshotsDir.getAbsolutePath(),
"db-wrong-name.yaml");
writeLocalDataToFile(localData, wrongPath);
-
- // Should throw IOException during init
- assertThrows(IOException.class, this::getNewOmSnapshotLocalDataManager);
+
+ localDataManager = getNewOmSnapshotLocalDataManager();
+
+ assertThat(localDataManager.getVersionNodeMapUnmodifiable()).isEmpty();
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
