From d8a2df25ad2a145712910259f4d4079c302c2aef Mon Sep 17 00:00:00 2001 From: bshashikant Date: Sun, 2 Aug 2020 21:59:56 +0530 Subject: [PATCH] HDFS-15498. Show snapshots deletion status in snapList cmd. (#2181) --- .../hadoop/hdfs/protocol/SnapshotStatus.java | 37 ++++++++++++++----- .../hdfs/protocolPB/PBHelperClient.java | 2 + .../src/main/proto/hdfs.proto | 1 + .../namenode/snapshot/SnapshotManager.java | 3 +- .../namenode/snapshot/TestListSnapshot.java | 14 ++++++- 5 files changed, 46 insertions(+), 11 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/SnapshotStatus.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/SnapshotStatus.java index 6a938a9e4b1..8c0dabd34ad 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/SnapshotStatus.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/SnapshotStatus.java @@ -40,6 +40,11 @@ public class SnapshotStatus { */ private final int snapshotID; + /** + * Whether the snapshot is deleted or not. + */ + private final boolean isDeleted; + /** * Full path of the parent. */ @@ -50,7 +55,7 @@ public class SnapshotStatus { EnumSet flags, String owner, String group, byte[] localName, long inodeId, int childrenNum, int snapshotID, - byte[] parentFullPath) { + boolean isDeleted, byte[] parentFullPath) { this.dirStatus = new HdfsFileStatus.Builder() .isdir(true) .mtime(modificationTime) @@ -64,13 +69,7 @@ public class SnapshotStatus { .children(childrenNum) .build(); this.snapshotID = snapshotID; - this.parentFullPath = parentFullPath; - } - - public SnapshotStatus(HdfsFileStatus dirStatus, - int snapshotNumber, byte[] parentFullPath) { - this.dirStatus = dirStatus; - this.snapshotID = snapshotNumber; + this.isDeleted = isDeleted; this.parentFullPath = parentFullPath; } @@ -89,6 +88,13 @@ public class SnapshotStatus { return snapshotID; } + /** + * @return whether snapshot is deleted + */ + public boolean isDeleted() { + return isDeleted; + } + /** * @return The basic information of the directory */ @@ -143,6 +149,7 @@ public class SnapshotStatus { + "%" + maxLen + "s " + "%s " // mod time + "%" + maxSnapshotID + "s " + + "%s " // deletion status + "%s"; // path SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm"); @@ -155,6 +162,7 @@ public class SnapshotStatus { String.valueOf(status.dirStatus.getLen()), dateFormat.format(new Date(status.dirStatus.getModificationTime())), status.snapshotID, + status.isDeleted ? "DELETED" : "ACTIVE", getSnapshotPath(DFSUtilClient.bytes2String(status.parentFullPath), status.dirStatus.getLocalName()) ); @@ -166,6 +174,9 @@ public class SnapshotStatus { return Math.max(n, String.valueOf(value).length()); } + /** + * To be used to for collection of snapshot jmx. + */ public static class Bean { private final String path; private final int snapshotID; @@ -173,15 +184,19 @@ public class SnapshotStatus { private final short permission; private final String owner; private final String group; + private final boolean isDeleted; + public Bean(String path, int snapshotID, long - modificationTime, short permission, String owner, String group) { + modificationTime, short permission, String owner, String group, + boolean isDeleted) { this.path = path; this.snapshotID = snapshotID; this.modificationTime = modificationTime; this.permission = permission; this.owner = owner; this.group = group; + this.isDeleted = isDeleted; } public String getPath() { @@ -207,6 +222,10 @@ public class SnapshotStatus { public String getGroup() { return group; } + + public boolean isDeleted() { + return isDeleted; + } } static String getSnapshotPath(String snapshottableDir, diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java index efe020a3acf..f3d0cd96ed1 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java @@ -1711,6 +1711,7 @@ public class PBHelperClient { status.getFileId(), status.getChildrenNum(), sdirStatusProto.getSnapshotID(), + sdirStatusProto.getIsDeleted(), sdirStatusProto.getParentFullpath().toByteArray()); } @@ -2425,6 +2426,7 @@ public class PBHelperClient { .newBuilder() .setSnapshotID(status.getSnapshotID()) .setParentFullpath(parentFullPathBytes) + .setIsDeleted(status.isDeleted()) .setDirStatus(fs); return builder.build(); } diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/hdfs.proto b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/hdfs.proto index 3e24d73ce2d..2440a7ac16d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/hdfs.proto +++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/proto/hdfs.proto @@ -574,6 +574,7 @@ message SnapshotStatusProto { // Fields specific for snapshot directory required uint32 snapshotID = 2; required bytes parent_fullpath = 3; + required bool isDeleted = 4; } 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 c908463d434..d56611235a9 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 @@ -599,7 +599,8 @@ public class SnapshotManager implements SnapshotStatsMXBean { // It is expensive to build the snapshot tree for the directory // and determine the child count. dir.getChildrenNum(Snapshot.CURRENT_STATE_ID), - s.getId(), DFSUtil.string2Bytes(dir.getParent().getFullPathName())); + s.getId(), s.getRoot().isMarkedAsDeleted(), + DFSUtil.string2Bytes(dir.getParent().getFullPathName())); } return statuses; diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestListSnapshot.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestListSnapshot.java index dcff2f7f7c6..672f21aed37 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestListSnapshot.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestListSnapshot.java @@ -30,7 +30,10 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import static org.apache.hadoop.hdfs.server.namenode.snapshot.SnapshotManager. + DFS_NAMENODE_SNAPSHOT_DELETION_ORDERED; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; @@ -51,6 +54,7 @@ public class TestListSnapshot { @Before public void setUp() throws Exception { conf = new Configuration(); + conf.setBoolean(DFS_NAMENODE_SNAPSHOT_DELETION_ORDERED, true); cluster = new MiniDFSCluster.Builder(conf).numDataNodes(REPLICATION) .build(); cluster.waitActive(); @@ -128,7 +132,15 @@ public class TestListSnapshot { snapshotStatuses[2].getFullPath()); hdfs.deleteSnapshot(dir1, "s2"); snapshotStatuses = hdfs.getSnapshotListing(dir1); - // There are now 2 snapshots for dir1 + // There are now 2 active snapshots for dir1 and one is marked deleted + assertEquals(3, snapshotStatuses.length); + assertTrue(snapshotStatuses[2].isDeleted()); + assertFalse(snapshotStatuses[1].isDeleted()); + assertFalse(snapshotStatuses[0].isDeleted()); + // delete the 1st snapshot + hdfs.deleteSnapshot(dir1, "s0"); + snapshotStatuses = hdfs.getSnapshotListing(dir1); + // There are now 2 snapshots now as the 1st one is deleted in order assertEquals(2, snapshotStatuses.length); } } \ No newline at end of file