Skip to content

HDDS-16465. Validate snapshot chains before mutation #11253

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
smengcl merged 2 commits into from
Sep 21, 2026
+146 −62
Merged

HDDS-16465. Validate snapshot chains before mutation #11253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
febbb05
HDDS-16465. Validate snapshot chains before mutation
andyhuangdev Sep 17, 2026
97e8dff
Merge remote-tracking branch 'origin/master' into HDDS-16465
adoroszlai Sep 19, 2026
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
163 changes: 101 additions & 62 deletions ...op-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand All @@ -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);
Expand All @@ -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)))) {
Expand All @@ -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)
Expand All @@ -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);
Expand All @@ -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) &&
Expand All @@ -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)
Expand Down Expand Up @@ -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(),
Expand All @@ -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;
}

/**
Expand Down
45 changes: 45 additions & 0 deletions ...ne/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotChain.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down