This is an automated email from the ASF dual-hosted git repository.
adoroszlai 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 07c71f1ca9b HDDS-16954. NoSuchElementException in SnapshotChainManager
if snapshot is purged during iteration (#11500)
07c71f1ca9b is described below
commit 07c71f1ca9b4d234e19de47ff47e3447577ceb56
Author: Siyao Meng <[email protected]>
AuthorDate: Tue Oct 13 15:15:37 2026 -0700
HDDS-16954. NoSuchElementException in SnapshotChainManager if snapshot is
purged during iteration (#11500)
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
---
.../hadoop/ozone/om/SnapshotChainManager.java | 30 ++++++++++++++--------
.../ozone/om/snapshot/TestSnapshotChain.java | 29 +++++++++++++++++++++
2 files changed, 48 insertions(+), 11 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 360f1f86b05..5dc6d1dbfb2 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
@@ -454,18 +454,26 @@ public boolean hasNext() {
@Override
public UUID next() {
- try {
- UUID prevSnapshotId = currentSnapshotId;
- if (reverse && hasPreviousGlobalSnapshot(currentSnapshotId) ||
- !reverse && hasNextGlobalSnapshot(currentSnapshotId)) {
- currentSnapshotId =
- reverse ? previousGlobalSnapshot(currentSnapshotId) :
nextGlobalSnapshot(currentSnapshotId);
- } else {
- currentSnapshotId = null;
+ // Hold the same lock as deleteSnapshot so that the chain cannot
change while advancing.
+ synchronized (SnapshotChainManager.this) {
+ try {
+ UUID prevSnapshotId = currentSnapshotId;
+ if (prevSnapshotId == null) {
+ throw new NoSuchElementException("No more snapshots in the
snapshot chain.");
+ }
+ // The snapshot may have been purged since the iterator moved to
it, in which case the walk ends here.
+ if (globalSnapshotChain.containsKey(prevSnapshotId) &&
+ (reverse && hasPreviousGlobalSnapshot(prevSnapshotId) ||
+ !reverse && hasNextGlobalSnapshot(prevSnapshotId))) {
+ currentSnapshotId =
+ reverse ? previousGlobalSnapshot(prevSnapshotId) :
nextGlobalSnapshot(prevSnapshotId);
+ } else {
+ currentSnapshotId = null;
+ }
+ return prevSnapshotId;
+ } catch (IOException e) {
+ throw new UncheckedIOException("Error while getting next snapshot
for " + currentSnapshotId, e);
}
- return prevSnapshotId;
- } catch (IOException e) {
- throw new UncheckedIOException("Error while getting next snapshot
for " + currentSnapshotId, e);
}
}
};
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 4534e4b7d3a..b73706d01ce 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
@@ -30,6 +30,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
@@ -380,6 +381,34 @@ public void testSnapshotChainIterator(int
numberOfSnapshots) throws IOException
}
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testSnapshotChainIteratorWhenNextSnapshotIsPurged(boolean
reverse) throws IOException {
+ Table<String, SnapshotInfo> snapshotInfo =
omMetadataManager.getSnapshotInfoTable();
+ List<SnapshotInfo> snapshotInfoList = new ArrayList<>();
+ UUID prevSnapshotID = null;
+ long time = System.currentTimeMillis();
+ for (int i = 0; i < 3; i++) {
+ UUID snapshotID = UUID.randomUUID();
+ SnapshotInfo snapInfo = createSnapshotInfo(snapshotID, prevSnapshotID,
prevSnapshotID, time++);
+ snapshotInfo.put(snapshotID.toString(), snapInfo);
+ prevSnapshotID = snapshotID;
+ snapshotInfoList.add(snapInfo);
+ }
+ chainManager = new SnapshotChainManager(omMetadataManager);
+ Iterator<UUID> iterator = chainManager.iterator(reverse);
+ assertEquals(snapshotInfoList.get(reverse ? 2 : 0).getSnapshotId(),
iterator.next());
+
+ // Purge the snapshot the iterator moved to, as a concurrent snapshot
purge would.
+ SnapshotInfo purged = snapshotInfoList.get(1);
+ assertTrue(chainManager.deleteSnapshot(purged));
+
+ assertTrue(iterator.hasNext());
+ assertEquals(purged.getSnapshotId(), iterator.next());
+ assertFalse(iterator.hasNext());
+ assertThrows(NoSuchElementException.class, iterator::next);
+ }
+
private static Stream<? extends Arguments> invalidSnapshotChain() {
List<UUID> nodes = IntStream.range(0, 5)
.mapToObj(i -> UUID.randomUUID())
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
