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 aed7ceac7cc HDDS-16465. Validate snapshot chains before mutation
(#11253)
aed7ceac7cc is described below
commit aed7ceac7cc515a796863b79066c9d5389732979
Author: Andy Huang <[email protected]>
AuthorDate: Tue Sep 22 06:30:07 2026 +0800
HDDS-16465. Validate snapshot chains before mutation (#11253)
---
.../hadoop/ozone/om/SnapshotChainManager.java | 163 +++++++++++++--------
.../ozone/om/snapshot/TestSnapshotChain.java | 45 ++++++
2 files changed, 146 insertions(+), 62 deletions(-)
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java
index 5951c1303ff..360f1f86b05 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java
@@ -70,18 +70,19 @@ public SnapshotChainManager(OMMetadataManager
metadataManager) {
}
/**
- * Add snapshot to global snapshot chain.
+ * Validate adding a snapshot to the global snapshot chain.
*/
- private void addSnapshotGlobal(UUID snapshotID, UUID prevGlobalID)
+ private void validateAddSnapshotGlobal(UUID snapshotID, UUID prevGlobalID)
throws IOException {
if (globalSnapshotChain.containsKey(snapshotID)) {
throw new IOException(String.format(
"Global Snapshot chain corruption. Snapshot with snapshotId: %s is "
+
"already present in the chain.", snapshotID));
}
+
if (!globalSnapshotChain.isEmpty() && prevGlobalID == null) {
throw new IOException(String.format("Snapshot chain " +
- "corruption. Adding snapshot %s as head node while there are %d " +
+ "corruption. Adding snapshot %s as head node while there are %d
" +
"snapshots in the global snapshot chain.", snapshotID,
globalSnapshotChain.size()));
}
@@ -94,16 +95,22 @@ private void addSnapshotGlobal(UUID snapshotID, UUID
prevGlobalID)
"snapshot chain.", prevGlobalID, snapshotID));
}
+ if (prevGlobalID != null &&
globalSnapshotChain.get(prevGlobalID).hasNextSnapshotId()) {
+ throw new IOException(String.format(
+ "Global Snapshot chain corruption. Snapshot with snapshotId: %s " +
+ "already has the next snapshotId: %s. Adding snapshot %s " +
+ "with prevSnapshotId: %s will make the chain non linear.",
+ prevGlobalID,
+ globalSnapshotChain.get(prevGlobalID).getNextSnapshotId(),
+ snapshotID, prevGlobalID));
+ }
+ }
+
+ /**
+ * Add snapshot to global snapshot chain.
+ */
+ private void addSnapshotGlobal(UUID snapshotID, UUID prevGlobalID) {
if (prevGlobalID != null) {
- if (globalSnapshotChain.get(prevGlobalID).hasNextSnapshotId()) {
- throw new IOException(String.format(
- "Global Snapshot chain corruption. Snapshot with snapshotId: %s " +
- "already has the next snapshotId: %s. Adding snapshot %s " +
- "with prevSnapshotId: %s will make the chain non linear.",
- prevGlobalID,
- globalSnapshotChain.get(prevGlobalID).getNextSnapshotId(),
- snapshotID, prevGlobalID));
- }
// On add snapshot, set previous snapshot entry nextSnapshotID =
// snapshotID
globalSnapshotChain.get(prevGlobalID).setNextSnapshotId(snapshotID);
@@ -119,11 +126,9 @@ private void addSnapshotGlobal(UUID snapshotID, UUID
prevGlobalID)
}
/**
- * Add snapshot to bucket snapshot chain(path based).
+ * Validate adding a snapshot to the path snapshot chain.
*/
- private void addSnapshotPath(String snapshotPath, UUID snapshotID,
- UUID prevPathID) throws IOException {
- // On add snapshot, set previous snapshot entry nextSnapshotId = snapshotId
+ private void validateAddSnapshotPath(String snapshotPath, UUID snapshotID,
UUID prevPathID) throws IOException {
if (prevPathID != null &&
((!snapshotChainByPath.containsKey(snapshotPath)) ||
(!snapshotChainByPath.get(snapshotPath).containsKey(prevPathID))))
{
@@ -142,17 +147,22 @@ private void addSnapshotPath(String snapshotPath, UUID
snapshotID,
snapshotChainByPath.get(snapshotPath).size()));
}
+ if (prevPathID != null &&
snapshotChainByPath.get(snapshotPath).get(prevPathID).hasNextSnapshotId()) {
+ throw new IOException(String.format(
+ "Path Snapshot chain corruption. Next snapshotId: %s is already " +
+ "set for snapshotId: %s. Adding snapshotId: %s with " +
+ "prevSnapshotId: %s will make the chain non linear.",
+ snapshotChainByPath.get(snapshotPath).get(prevPathID)
+ .getNextSnapshotId(), prevPathID,
+ snapshotID, prevPathID));
+ }
+ }
+
+ /**
+ * Add snapshot to bucket snapshot chain(path based).
+ */
+ private void addSnapshotPath(String snapshotPath, UUID snapshotID, UUID
prevPathID) {
if (prevPathID != null && snapshotChainByPath.containsKey(snapshotPath)) {
- if (snapshotChainByPath.get(snapshotPath).get(prevPathID)
- .hasNextSnapshotId()) {
- throw new IOException(String.format(
- "Path Snapshot chain corruption. Next snapshotId: %s is already " +
- "set for snapshotId: %s. Adding snapshotId: %s with " +
- "prevSnapshotId: %s will make the chain non linear.",
- snapshotChainByPath.get(snapshotPath).get(prevPathID)
- .getNextSnapshotId(), prevPathID,
- snapshotID, prevPathID));
- }
snapshotChainByPath
.get(snapshotPath)
.get(prevPathID)
@@ -170,26 +180,31 @@ private void addSnapshotPath(String snapshotPath, UUID
snapshotID,
latestSnapshotIdByPath.put(snapshotPath, snapshotID);
}
+ private void validateDeleteSnapshotGlobal(UUID snapshotID) throws
IOException {
+ UUID next = globalSnapshotChain.get(snapshotID).getNextSnapshotId();
+ UUID prev = globalSnapshotChain.get(snapshotID).getPreviousSnapshotId();
+ if (prev != null && !globalSnapshotChain.containsKey(prev)) {
+ throw new IOException(String.format(
+ "Global snapshot chain corruption. " +
+ "SnapshotId: %s to be deleted has previous snapshotId: %s " +
+ "but associated snapshot is not found in snapshot chain.",
+ snapshotID, prev));
+ }
+ if (next != null && !globalSnapshotChain.containsKey(next)) {
+ throw new IOException(String.format(
+ "Global snapshot chain corruption. " +
+ "SnapshotId: {%s} to be deleted has next snapshotId: %s " +
+ "but associated snapshot is not found in snapshot chain.",
+ snapshotID, next));
+ }
+ }
+
private boolean deleteSnapshotGlobal(UUID snapshotID) throws IOException {
if (globalSnapshotChain.containsKey(snapshotID)) {
// reset prev and next snapshot entries in chain ordered list
// for node removal
UUID next = globalSnapshotChain.get(snapshotID).getNextSnapshotId();
UUID prev = globalSnapshotChain.get(snapshotID).getPreviousSnapshotId();
- if (prev != null && !globalSnapshotChain.containsKey(prev)) {
- throw new IOException(String.format(
- "Global snapshot chain corruption. " +
- "SnapshotId: %s to be deleted has previous snapshotId: %s " +
- "but associated snapshot is not found in snapshot chain.",
- snapshotID, prev));
- }
- if (next != null && !globalSnapshotChain.containsKey(next)) {
- throw new IOException(String.format(
- "Global snapshot chain corruption. " +
- "SnapshotId: {%s} to be deleted has next snapshotId: %s " +
- "but associated snapshot is not found in snapshot chain.",
- snapshotID, next));
- }
globalSnapshotChain.remove(snapshotID);
if (next != null) {
globalSnapshotChain.get(next).setPreviousSnapshotId(prev);
@@ -213,6 +228,30 @@ private boolean deleteSnapshotGlobal(UUID snapshotID)
throws IOException {
}
}
+ private void validateDeleteSnapshotPath(String snapshotPath, UUID
snapshotId) throws IOException {
+ UUID nextSnapshotId =
snapshotChainByPath.get(snapshotPath).get(snapshotId).getNextSnapshotId();
+ UUID previousSnapshotId =
snapshotChainByPath.get(snapshotPath).get(snapshotId).getPreviousSnapshotId();
+ if (previousSnapshotId != null &&
+ !snapshotChainByPath.get(snapshotPath)
+ .containsKey(previousSnapshotId)) {
+ throw new IOException(String.format(
+ "Path snapshot chain corruption. " +
+ "SnapshotId: %s at snapshotPath: %s to be deleted has " +
+ "previous snapshotId: %s but associated snapshot is not " +
+ "found in snapshot chain.", snapshotId, snapshotPath,
+ previousSnapshotId));
+ }
+ if (nextSnapshotId != null && !snapshotChainByPath.get(snapshotPath)
+ .containsKey(nextSnapshotId)) {
+ throw new IOException(String.format(
+ "Path snapshot chain corruption. " +
+ "SnapshotId: %s at snapshotPath: %s to be deleted has next " +
+ "snapshotId: %s but associated snapshot is not found in " +
+ "snapshot chain.", snapshotId, snapshotPath,
+ nextSnapshotId));
+ }
+ }
+
private boolean deleteSnapshotPath(String snapshotPath,
UUID snapshotId) throws IOException {
if (snapshotChainByPath.containsKey(snapshotPath) &&
@@ -228,26 +267,6 @@ private boolean deleteSnapshotPath(String snapshotPath,
.get(snapshotId)
.getPreviousSnapshotId();
- if (previousSnapshotId != null &&
- !snapshotChainByPath.get(snapshotPath)
- .containsKey(previousSnapshotId)) {
- throw new IOException(String.format(
- "Path snapshot chain corruption. " +
- "SnapshotId: %s at snapshotPath: %s to be deleted has " +
- "previous snapshotId: %s but associated snapshot is not " +
- "found in snapshot chain.", snapshotId, snapshotPath,
- previousSnapshotId));
- }
- if (nextSnapshotId != null && !snapshotChainByPath.get(snapshotPath)
- .containsKey(nextSnapshotId)) {
- throw new IOException(String.format(
- "Path snapshot chain corruption. " +
- "SnapshotId: %s at snapshotPath: %s to be deleted has next " +
- "snapshotId: %s but associated snapshot is not found in " +
- "snapshot chain.", snapshotId, snapshotPath,
- nextSnapshotId));
- }
-
snapshotChainByPath.get(snapshotPath).remove(snapshotId);
if (nextSnapshotId != null) {
snapshotChainByPath.get(snapshotPath)
@@ -349,6 +368,9 @@ private boolean loadFromSnapshotInfoTable(OMMetadataManager
metadataManager) {
public synchronized void addSnapshot(SnapshotInfo snapshotInfo)
throws IOException {
validateSnapshotChain();
+ validateAddSnapshotGlobal(snapshotInfo.getSnapshotId(),
snapshotInfo.getGlobalPreviousSnapshotId());
+ validateAddSnapshotPath(snapshotInfo.getSnapshotPath(),
snapshotInfo.getSnapshotId(),
+ snapshotInfo.getPathPreviousSnapshotId());
addSnapshotGlobal(snapshotInfo.getSnapshotId(),
snapshotInfo.getGlobalPreviousSnapshotId());
addSnapshotPath(snapshotInfo.getSnapshotPath(),
@@ -375,8 +397,25 @@ public synchronized void updateSnapshot(SnapshotInfo
snapshotInfo) {
public synchronized boolean deleteSnapshot(SnapshotInfo snapshotInfo)
throws IOException {
validateSnapshotChain();
- return deleteSnapshotGlobal(snapshotInfo.getSnapshotId()) &&
- deleteSnapshotPath(snapshotInfo.getSnapshotPath(),
snapshotInfo.getSnapshotId());
+ UUID snapshotId = snapshotInfo.getSnapshotId();
+ String snapshotPath = snapshotInfo.getSnapshotPath();
+ boolean inGlobalChain = globalSnapshotChain.containsKey(snapshotId);
+ boolean inPathChain = snapshotChainByPath.containsKey(snapshotPath)
+ && snapshotChainByPath.get(snapshotPath).containsKey(snapshotId);
+ if (inGlobalChain != inPathChain) {
+ throw new IOException(String.format(
+ "Snapshot chain corruption. SnapshotId: %s has inconsistent
membership for snapshot path: %s.",
+ snapshotId, snapshotPath));
+ }
+ if (!inGlobalChain) {
+ LOG.warn("SnapshotId: {} is not found in the snapshot chain.",
snapshotId);
+ return false;
+ }
+ validateDeleteSnapshotGlobal(snapshotId);
+ validateDeleteSnapshotPath(snapshotPath, snapshotId);
+ deleteSnapshotGlobal(snapshotId);
+ deleteSnapshotPath(snapshotPath, snapshotId);
+ return true;
}
/**
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotChain.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotChain.java
index e62b6489325..4534e4b7d3a 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotChain.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotChain.java
@@ -207,6 +207,51 @@ public void testAddSnapshot() throws Exception {
}
}
+ @Test
+ public void testFailedAddPreservesChains() throws Exception {
+ UUID id = UUID.randomUUID();
+ SnapshotInfo info = createSnapshotInfo(id, UUID.randomUUID(), null, 1L);
+
+ assertThrows(IOException.class, () -> chainManager.addSnapshot(info));
+
+ assertTrue(chainManager.getGlobalSnapshotChain().isEmpty());
+ assertTrue(chainManager.getSnapshotChainByPath().isEmpty());
+ }
+
+ @Test
+ public void testFailedAddPreservesExistingSnapshot() throws Exception {
+ UUID firstId = UUID.randomUUID();
+ chainManager.addSnapshot(createSnapshotInfo(firstId, null, null, 1L));
+
+ UUID rejectedId = UUID.randomUUID();
+ SnapshotInfo rejected = createSnapshotInfo(rejectedId, UUID.randomUUID(),
firstId, 2L);
+
+ assertThrows(IOException.class, () -> chainManager.addSnapshot(rejected));
+
+ assertEquals(1, chainManager.getGlobalSnapshotChain().size());
+ assertEquals(1, chainManager.getSnapshotChainPath("vol1/bucket1").size());
+ assertEquals(firstId, chainManager.getOldestGlobalSnapshotId());
+ assertEquals(firstId, chainManager.getLatestGlobalSnapshotId());
+ assertEquals(firstId,
chainManager.getLatestPathSnapshotId("vol1/bucket1"));
+ assertFalse(chainManager.hasNextGlobalSnapshot(firstId));
+ assertFalse(chainManager.hasNextPathSnapshot("vol1/bucket1", firstId));
+ assertFalse(chainManager.getGlobalSnapshotChain().containsKey(rejectedId));
+ }
+
+ @Test
+ public void testWrongPathDeletePreservesGlobalChain() throws Exception {
+ UUID id = UUID.randomUUID();
+ chainManager.addSnapshot(createSnapshotInfo(id, null, null, 1L));
+
+ SnapshotInfo wrongPath = createSnapshotInfo(id, null, null, 1L);
+ wrongPath.setSnapshotPath("vol1/bucket2");
+
+ assertThrows(IOException.class, () ->
chainManager.deleteSnapshot(wrongPath));
+
+ assertTrue(chainManager.getGlobalSnapshotChain().containsKey(id));
+
assertTrue(chainManager.getSnapshotChainPath("vol1/bucket1").containsKey(id));
+ }
+
@Test
public void testDeleteSnapshot() throws Exception {
// add three snapshots
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
