From 41182a9b6d81d0c8a4dc0a9cf89ea0ade815afd3 Mon Sep 17 00:00:00 2001 From: Tsz-Wo Nicholas Sze Date: Thu, 27 Aug 2020 02:24:52 -0700 Subject: [PATCH] HDFS-15500. In-order deletion of snapshots: Diff lists must be update only in the last snapshot. (#2233) --- .../hadoop/hdfs/server/namenode/FSNamesystem.java | 6 ++++++ .../namenode/snapshot/AbstractINodeDiffList.java | 6 +++++- .../namenode/snapshot/DiffListByArrayList.java | 2 ++ .../snapshot/DirectoryWithSnapshotFeature.java | 2 ++ .../server/namenode/snapshot/SnapshotManager.java | 12 ++++++++++++ 5 files changed, 27 insertions(+), 1 deletion(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 15bf6b16b5..badf237049 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -1569,6 +1569,12 @@ public void checkOperation(OperationCategory op) throws StandbyException { // null in some unit tests haContext.checkOperation(op); } + + boolean assertsEnabled = false; + assert assertsEnabled = true; // Intentional side effect!!! + if (assertsEnabled && op == OperationCategory.WRITE) { + getSnapshotManager().initThreadLocals(); + } } /** diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java index 776adf1def..16e3b75369 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/AbstractINodeDiffList.java @@ -76,7 +76,11 @@ public final void deleteSnapshotDiff(INode.ReclaimContext reclaimContext, if (diffs == null) { return; } - int snapshotIndex = diffs.binarySearch(snapshot); + final int snapshotIndex = diffs.binarySearch(snapshot); + // DeletionOrdered: only can remove the element at index 0 and no prior + // check snapshotIndex <= 0 since the diff may not exist + assert !SnapshotManager.isDeletionOrdered() + || (snapshotIndex <= 0 && prior == Snapshot.NO_SNAPSHOT_ID); D removed; if (snapshotIndex == 0) { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DiffListByArrayList.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DiffListByArrayList.java index 95c23dfc02..7fa3f05efd 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DiffListByArrayList.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DiffListByArrayList.java @@ -57,6 +57,8 @@ public int size() { @Override public T remove(int i) { + // DeletionOrdered: only can remove the element at index 0 + assert !SnapshotManager.isDeletionOrdered() || i == 0; return list.remove(i); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectoryWithSnapshotFeature.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectoryWithSnapshotFeature.java index b8f7b65ea7..c3a9aa14e9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectoryWithSnapshotFeature.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectoryWithSnapshotFeature.java @@ -175,6 +175,8 @@ void combinePosteriorAndCollectBlocks( final INode.ReclaimContext reclaimContext, final INodeDirectory currentDir, final DirectoryDiff posterior) { + // DeletionOrdered: must not combine posterior + assert !SnapshotManager.isDeletionOrdered(); diff.combinePosterior(posterior.diff, new Diff.Processor() { /** Collect blocks for deleted files. */ @Override diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java index b5b0971298..789fa3fa8b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/SnapshotManager.java @@ -95,6 +95,18 @@ public class SnapshotManager implements SnapshotStatsMXBean { static final long DFS_NAMENODE_SNAPSHOT_DELETION_ORDERED_GC_PERIOD_MS_DEFAULT = 5 * 60_000L; //5 minutes + private static final ThreadLocal DELETION_ORDERED + = new ThreadLocal<>(); + + static boolean isDeletionOrdered() { + final Boolean b = DELETION_ORDERED.get(); + return b != null? b: false; + } + + public void initThreadLocals() { + DELETION_ORDERED.set(isSnapshotDeletionOrdered()); + } + private final FSDirectory fsdir; private boolean captureOpenFiles; /**