Skip to content
Merged
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7327b47
HDDS-12559. Implement Bulk Ozone Locks for taking locks on multiple s…
swamirishi Mar 11, 2025
bd5b0c6
HDDS-12560. Reclaimable Filter for Snaphost Garbage Collections
swamirishi Mar 11, 2025
f1c85fd
HDDS-12560. Mock SnapshotDiffManager construction
swamirishi Mar 12, 2025
43ab7b7
HDDS-12559. Revert unintended change in method signature
swamirishi Mar 12, 2025
51c88f1
Merge remote-tracking branch 'origin/HDDS-12559' into HEAD
swamirishi Mar 12, 2025
b901166
HDDS-12559. Address review comments
swamirishi Apr 1, 2025
690eae9
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi Apr 1, 2025
865f3a5
HDDS-12559. Add javadoc
swamirishi Apr 1, 2025
5743edb
HDDS-12560. Address review comments
swamirishi Apr 1, 2025
abcfaff
HDDS-12559. Move acquireLock to another private function
swamirishi Apr 1, 2025
a2127a4
HDDS-12560. Address review comments
swamirishi Apr 1, 2025
9f6d2a0
HDDS-12559. Address review comments
swamirishi Apr 2, 2025
9d6bae3
HDDS-12560. Address review comments
swamirishi Apr 3, 2025
7d785ab
Merge remote-tracking branch 'origin/HDDS-12559' into HEAD
swamirishi Apr 3, 2025
e1d2317
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi Apr 3, 2025
93ba939
HDDS-12560. Address review comments
swamirishi Apr 17, 2025
3935bf8
Update hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozo…
swamirishi Apr 17, 2025
6d638a0
HDDS-12560. Fix method folding
swamirishi Apr 17, 2025
681bb3d
Merge remote-tracking branch 'apache/master' into HEAD
swamirishi Apr 17, 2025
8fd746c
HDDS-12560. Remove Checked Function
swamirishi Apr 17, 2025
b69182e
Merge remote-tracking branch 'origin/HDDS-12560' into HEAD
swamirishi Apr 17, 2025
d32bcf0
Update hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozo…
swamirishi Apr 17, 2025
f8a2b6e
HDDS-12560. Fix compilation issue
swamirishi Apr 17, 2025
f323e3e
Merge remote-tracking branch 'origin/HDDS-12560' into HEAD
swamirishi Apr 17, 2025
29a26d2
HDDS-12560. Address review comments
swamirishi Apr 30, 2025
1ebba73
HDDS-12560. Fix test case
swamirishi Apr 30, 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
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@

package org.apache.hadoop.ozone.om.snapshot.filter;

import com.google.common.collect.Lists;
import java.io.Closeable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.UUID;
import java.util.stream.Collectors;
import org.apache.hadoop.hdds.utils.IOUtils;
import org.apache.hadoop.hdds.utils.db.Table;
import org.apache.hadoop.ozone.om.KeyManager;
Expand Down Expand Up @@ -56,7 +55,8 @@ public abstract class ReclaimableFilter<V> implements CheckedFunction<Table.KeyV
private final SnapshotInfo currentSnapshotInfo;
private final OmSnapshotManager omSnapshotManager;
private final SnapshotChainManager snapshotChainManager;

private final List<SnapshotInfo> tmpValidationSnapshotInfos;
private final List<UUID> lockedSnapshotIds;
private final List<SnapshotInfo> previousSnapshotInfos;
private final List<ReferenceCounted<OmSnapshot>> previousOmSnapshots;
private final MultiSnapshotLocks snapshotIdLocks;
Expand Down Expand Up @@ -93,6 +93,9 @@ public ReclaimableFilter(OzoneManager ozoneManager, OmSnapshotManager omSnapshot
this.numberOfPreviousSnapshotsFromChain = numberOfPreviousSnapshotsFromChain;
this.previousOmSnapshots = new ArrayList<>(numberOfPreviousSnapshotsFromChain);
this.previousSnapshotInfos = new ArrayList<>(numberOfPreviousSnapshotsFromChain);
// Used for tmp list to avoid lots of garbage collection of list.
this.tmpValidationSnapshotInfos = new ArrayList<>(numberOfPreviousSnapshotsFromChain);
this.lockedSnapshotIds = new ArrayList<>(numberOfPreviousSnapshotsFromChain + 1);
}

private List<SnapshotInfo> getLastNSnapshotInChain(String volume, String bucket) throws IOException {
Expand All @@ -101,36 +104,42 @@ private List<SnapshotInfo> getLastNSnapshotInChain(String volume, String bucket)
throw new IOException("Volume and Bucket name for snapshot : " + currentSnapshotInfo + " do not match " +
"against the volume: " + volume + " and bucket: " + bucket + " of the key.");
}
tmpValidationSnapshotInfos.clear();
SnapshotInfo expectedPreviousSnapshotInfo = currentSnapshotInfo == null
? SnapshotUtils.getLatestSnapshotInfo(volume, bucket, ozoneManager, snapshotChainManager)
: SnapshotUtils.getPreviousSnapshot(ozoneManager, snapshotChainManager, currentSnapshotInfo);
List<SnapshotInfo> snapshotInfos = Lists.newArrayList();
SnapshotInfo snapshotInfo = expectedPreviousSnapshotInfo;
while (snapshotInfos.size() < numberOfPreviousSnapshotsFromChain) {
while (tmpValidationSnapshotInfos.size() < numberOfPreviousSnapshotsFromChain) {
Copy link
Member

Choose a reason for hiding this comment

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

If snapshotInfo is null there is no previous snapshot, Can we terminate this loop early?

Suggested change
while (tmpValidationSnapshotInfos.size() < numberOfPreviousSnapshotsFromChain) {
while (snapshotInfo != null && tmpValidationSnapshotInfos.size() < numberOfPreviousSnapshotsFromChain) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even if the snapshot info is null. We still want the previous SnapshotInfos list to be equal to the number of numberOfPreviousSnapshotsFromChain. When the snapshotInfo is null, we want the list to be filled with the required null values.

// If changes made to the snapshot have not been flushed to disk, throw exception immediately, next run of
// garbage collection would process the snapshot.
if (!OmSnapshotManager.areSnapshotChangesFlushedToDB(ozoneManager.getMetadataManager(), snapshotInfo)) {
throw new IOException("Changes made to the snapshot " + snapshotInfo + " have not been flushed to the disk ");
}
snapshotInfos.add(snapshotInfo);
tmpValidationSnapshotInfos.add(snapshotInfo);
snapshotInfo = snapshotInfo == null ? null
: SnapshotUtils.getPreviousSnapshot(ozoneManager, snapshotChainManager, snapshotInfo);
}

// Reversing list to get the correct order in chain. To ensure locking order is as per the chain ordering.
Collections.reverse(snapshotInfos);
return snapshotInfos;
Collections.reverse(tmpValidationSnapshotInfos);
return tmpValidationSnapshotInfos;
}

private boolean validateExistingLastNSnapshotsInChain(String volume, String bucket) throws IOException {
List<SnapshotInfo> expectedLastNSnapshotsInChain = getLastNSnapshotInChain(volume, bucket);
List<UUID> expectedSnapshotIds = expectedLastNSnapshotsInChain.stream()
.map(snapshotInfo -> snapshotInfo == null ? null : snapshotInfo.getSnapshotId())
.collect(Collectors.toList());
List<UUID> existingSnapshotIds = previousOmSnapshots.stream()
.map(omSnapshotReferenceCounted -> omSnapshotReferenceCounted == null ? null :
omSnapshotReferenceCounted.get().getSnapshotID()).collect(Collectors.toList());
return expectedSnapshotIds.equals(existingSnapshotIds);
if (expectedLastNSnapshotsInChain.size() != previousOmSnapshots.size()) {
return false;
}
for (int i = 0; i < expectedLastNSnapshotsInChain.size(); i++) {
SnapshotInfo snapshotInfo = expectedLastNSnapshotsInChain.get(i);
ReferenceCounted<OmSnapshot> omSnapshot = previousOmSnapshots.get(i);
UUID snapshotId = snapshotInfo == null ? null : snapshotInfo.getSnapshotId();
UUID existingOmSnapshotId = omSnapshot == null ? null : omSnapshot.get().getSnapshotID();
if (!Objects.equals(snapshotId, existingOmSnapshotId)) {
return false;
}
}
return true;
}

// Initialize the last N snapshots in the chain by acquiring locks. Throw IOException if it fails.
Expand All @@ -139,35 +148,34 @@ private void initializePreviousSnapshotsFromChain(String volume, String bucket)
try {
// Acquire lock on last N snapshot & current snapshot(AOS if it is null).
List<SnapshotInfo> expectedLastNSnapshotsInChain = getLastNSnapshotInChain(volume, bucket);
List<UUID> lockIds = expectedLastNSnapshotsInChain.stream()
.map(snapshotInfo -> snapshotInfo == null ? null : snapshotInfo.getSnapshotId())
.collect(Collectors.toList());
for (SnapshotInfo snapshotInfo : expectedLastNSnapshotsInChain) {
lockedSnapshotIds.add(snapshotInfo == null ? null : snapshotInfo.getSnapshotId());
}
//currentSnapshotInfo for AOS will be null.
lockIds.add(currentSnapshotInfo == null ? null : currentSnapshotInfo.getSnapshotId());

if (snapshotIdLocks.acquireLock(lockIds).isLockAcquired()) {
for (SnapshotInfo snapshotInfo : expectedLastNSnapshotsInChain) {
if (snapshotInfo != null) {
// Fail operation if any of the previous snapshots are not active.
previousOmSnapshots.add(omSnapshotManager.getActiveSnapshot(snapshotInfo.getVolumeName(),
snapshotInfo.getBucketName(), snapshotInfo.getName()));
previousSnapshotInfos.add(snapshotInfo);
} else {
previousOmSnapshots.add(null);
previousSnapshotInfos.add(null);
}

// NOTE: Getting volumeId and bucket from active OM. This would be wrong on volume & bucket renames
// support.
bucketInfo = ozoneManager.getBucketInfo(volume, bucket);
volumeId = ozoneManager.getMetadataManager().getVolumeId(volume);
}
} else {
lockedSnapshotIds.add(currentSnapshotInfo == null ? null : currentSnapshotInfo.getSnapshotId());

if (!snapshotIdLocks.acquireLock(lockedSnapshotIds).isLockAcquired()) {
throw new IOException("Lock acquisition failed for last N snapshots: " +
expectedLastNSnapshotsInChain + ", " + currentSnapshotInfo);
}
for (SnapshotInfo snapshotInfo : expectedLastNSnapshotsInChain) {
if (snapshotInfo != null) {
// Fail operation if any of the previous snapshots are not active.
previousOmSnapshots.add(omSnapshotManager.getActiveSnapshot(snapshotInfo.getVolumeName(),
snapshotInfo.getBucketName(), snapshotInfo.getName()));
previousSnapshotInfos.add(snapshotInfo);
} else {
previousOmSnapshots.add(null);
previousSnapshotInfos.add(null);
}

// NOTE: Getting volumeId and bucket from active OM. This would be wrong on volume & bucket renames
// support.
bucketInfo = ozoneManager.getBucketInfo(volume, bucket);
volumeId = ozoneManager.getMetadataManager().getVolumeId(volume);
}
} catch (IOException e) {
this.close();
this.cleanup();
throw e;
}
}
Expand All @@ -194,10 +202,15 @@ public synchronized Boolean apply(Table.KeyValue<String, V> keyValue) throws IOE

@Override
public void close() throws IOException {
this.cleanup();
}

private void cleanup() {
this.snapshotIdLocks.releaseLock();
IOUtils.close(LOG, previousOmSnapshots);
previousOmSnapshots.clear();
previousSnapshotInfos.clear();
lockedSnapshotIds.clear();
}

protected ReferenceCounted<OmSnapshot> getPreviousOmSnapshot(int index) {
Expand Down