From b25e94ce29b311a37334317d72e46373b256c111 Mon Sep 17 00:00:00 2001 From: Takanobu Asanuma Date: Fri, 22 Nov 2019 13:48:02 +0900 Subject: [PATCH] HDFS-14924. RenameSnapshot not updating new modification time. Contributed by hemanthboyina --- .../hdfs/server/namenode/FSDirSnapshotOp.java | 6 +++-- .../hdfs/server/namenode/FSEditLog.java | 12 ++++++++-- .../hdfs/server/namenode/FSEditLogLoader.java | 2 +- .../hdfs/server/namenode/FSEditLogOp.java | 22 ++++++++++++++++-- .../hdfs/server/namenode/INodeDirectory.java | 14 ++++++++--- .../DirectorySnapshottableFeature.java | 5 +++- .../namenode/snapshot/SnapshotManager.java | 5 ++-- .../namenode/snapshot/TestSnapshot.java | 18 ++++++++++++++ .../src/test/resources/editsStored | Bin 7917 -> 7925 bytes .../src/test/resources/editsStored.xml | 1 + 10 files changed, 72 insertions(+), 13 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java index 49020e5948..6309461c95 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirSnapshotOp.java @@ -129,15 +129,17 @@ static void renameSnapshot(FSDirectory fsd, FSPermissionChecker pc, fsd.checkOwner(pc, iip); } verifySnapshotName(fsd, snapshotNewName, path); + // time of snapshot modification + final long now = Time.now(); fsd.writeLock(); try { snapshotManager.renameSnapshot(iip, path, snapshotOldName, - snapshotNewName); + snapshotNewName, now); } finally { fsd.writeUnlock(); } fsd.getEditLog().logRenameSnapshot(path, snapshotOldName, - snapshotNewName, logRetryCache); + snapshotNewName, logRetryCache, now); } static SnapshottableDirectoryStatus[] getSnapshottableDirListing( diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java index ae7101d5d2..dca41cd12c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLog.java @@ -1140,11 +1140,19 @@ void logDeleteSnapshot(String snapRoot, String snapName, boolean toLogRpcIds) { logEdit(op); } + /** + * Log that a snapshot is renamed. + * @param path Root of the snapshot. + * @param snapOldName Old name of the snapshot. + * @param snapNewName New name the snapshot will be renamed to. + * @param toLogRpcIds If it is logging RPC ids. + * @param mtime The snapshot modification time set by Time.now(). + */ void logRenameSnapshot(String path, String snapOldName, String snapNewName, - boolean toLogRpcIds) { + boolean toLogRpcIds, long mtime) { RenameSnapshotOp op = RenameSnapshotOp.getInstance(cache.get()) .setSnapshotRoot(path).setSnapshotOldName(snapOldName) - .setSnapshotNewName(snapNewName); + .setSnapshotNewName(snapNewName).setSnapshotMTime(mtime); logRpcIds(op, toLogRpcIds); logEdit(op); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java index 5ed869ebd1..39ebfc872c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java @@ -841,7 +841,7 @@ private long applyEditLogOp(FSEditLogOp op, FSDirectory fsDir, INodesInPath iip = fsDir.getINodesInPath(snapshotRoot, DirOp.WRITE); fsNamesys.getSnapshotManager().renameSnapshot(iip, snapshotRoot, renameSnapshotOp.snapshotOldName, - renameSnapshotOp.snapshotNewName); + renameSnapshotOp.snapshotNewName, renameSnapshotOp.mtime); if (toAddRetryCache) { fsNamesys.addCacheEntry(renameSnapshotOp.rpcClientId, diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java index e7f4dcbfe2..46d4a5ec0e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogOp.java @@ -3606,6 +3606,9 @@ static class RenameSnapshotOp extends FSEditLogOp { String snapshotRoot; String snapshotOldName; String snapshotNewName; + /** Modification time of the edit set by Time.now(). */ + long mtime; + RenameSnapshotOp() { super(OP_RENAME_SNAPSHOT); @@ -3620,13 +3623,16 @@ void resetSubFields() { snapshotRoot = null; snapshotOldName = null; snapshotNewName = null; + mtime = 0L; } - + + /* set the old name of the snapshot. */ RenameSnapshotOp setSnapshotOldName(String snapshotOldName) { this.snapshotOldName = snapshotOldName; return this; } + /* set the new name of the snapshot. */ RenameSnapshotOp setSnapshotNewName(String snapshotNewName) { this.snapshotNewName = snapshotNewName; return this; @@ -3637,11 +3643,18 @@ RenameSnapshotOp setSnapshotRoot(String snapshotRoot) { return this; } + /* The snapshot rename time set by Time.now(). */ + RenameSnapshotOp setSnapshotMTime(long mTime) { + this.mtime = mTime; + return this; + } + @Override void readFields(DataInputStream in, int logVersion) throws IOException { snapshotRoot = FSImageSerialization.readString(in); snapshotOldName = FSImageSerialization.readString(in); snapshotNewName = FSImageSerialization.readString(in); + mtime = FSImageSerialization.readLong(in); // read RPC ids if necessary readRpcIds(in, logVersion); @@ -3652,6 +3665,7 @@ public void writeFields(DataOutputStream out) throws IOException { FSImageSerialization.writeString(snapshotRoot, out); FSImageSerialization.writeString(snapshotOldName, out); FSImageSerialization.writeString(snapshotNewName, out); + FSImageSerialization.writeLong(mtime, out); writeRpcIds(rpcClientId, rpcCallId, out); } @@ -3661,6 +3675,7 @@ protected void toXml(ContentHandler contentHandler) throws SAXException { XMLUtils.addSaxString(contentHandler, "SNAPSHOTROOT", snapshotRoot); XMLUtils.addSaxString(contentHandler, "SNAPSHOTOLDNAME", snapshotOldName); XMLUtils.addSaxString(contentHandler, "SNAPSHOTNEWNAME", snapshotNewName); + XMLUtils.addSaxString(contentHandler, "MTIME", Long.toString(mtime)); appendRpcIdsToXml(contentHandler, rpcClientId, rpcCallId); } @@ -3669,6 +3684,7 @@ void fromXml(Stanza st) throws InvalidXmlException { snapshotRoot = st.getValue("SNAPSHOTROOT"); snapshotOldName = st.getValue("SNAPSHOTOLDNAME"); snapshotNewName = st.getValue("SNAPSHOTNEWNAME"); + this.mtime = Long.parseLong(st.getValue("MTIME")); readRpcIdsFromXml(st); } @@ -3681,7 +3697,9 @@ public String toString() { .append(", snapshotOldName=") .append(snapshotOldName) .append(", snapshotNewName=") - .append(snapshotNewName); + .append(snapshotNewName) + .append(", mtime=") + .append(mtime); appendRpcIdsToString(builder, rpcClientId, rpcCallId); builder.append("]"); return builder.toString(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java index dd3c22c1cf..cf85978920 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java @@ -296,9 +296,17 @@ public Snapshot removeSnapshot( reclaimContext, this, snapshotName); } - public void renameSnapshot(String path, String oldName, String newName) - throws SnapshotException { - getDirectorySnapshottableFeature().renameSnapshot(path, oldName, newName); + /** + * Rename a snapshot. + * @param path The directory path where the snapshot was taken. + * @param oldName Old name of the snapshot + * @param newName New name the snapshot will be renamed to + * @param mtime The snapshot modification time set by Time.now(). + */ + public void renameSnapshot(String path, String oldName, String newName, + long mtime) throws SnapshotException { + getDirectorySnapshottableFeature().renameSnapshot(path, oldName, newName, + mtime); } /** add DirectorySnapshottableFeature */ diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectorySnapshottableFeature.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectorySnapshottableFeature.java index b98b9cd5b6..4049f2f167 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectorySnapshottableFeature.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/snapshot/DirectorySnapshottableFeature.java @@ -112,12 +112,14 @@ public ReadOnlyList getSnapshotList() { * Old name of the snapshot * @param newName * New name the snapshot will be renamed to + * @param mtime The snapshot modification time set by Time.now(). * @throws SnapshotException * Throw SnapshotException when either the snapshot with the old * name does not exist or a snapshot with the new name already * exists */ - public void renameSnapshot(String path, String oldName, String newName) + public void renameSnapshot(String path, String oldName, String newName, + long mtime) throws SnapshotException { final int indexOfOld = searchSnapshot(DFSUtil.string2Bytes(oldName)); if (indexOfOld < 0) { @@ -137,6 +139,7 @@ public void renameSnapshot(String path, String oldName, String newName) Snapshot snapshot = snapshotsByNames.remove(indexOfOld); final INodeDirectory ssRoot = snapshot.getRoot(); ssRoot.setLocalName(newNameBytes); + ssRoot.setModificationTime(mtime, Snapshot.CURRENT_STATE_ID); indexOfNew = -indexOfNew - 1; if (indexOfNew <= indexOfOld) { snapshotsByNames.add(indexOfNew, snapshot); 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 9244710ed3..7a4e7a04d6 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 @@ -362,6 +362,7 @@ public void deleteSnapshot(final INodesInPath iip, final String snapshotName, * Old name of the snapshot * @param newSnapshotName * New name of the snapshot + * @param now is the snapshot modification time set by Time.now(). * @throws IOException * Throw IOException when 1) the given path does not lead to an * existing snapshottable directory, and/or 2) the snapshot with the @@ -369,10 +370,10 @@ public void deleteSnapshot(final INodesInPath iip, final String snapshotName, * a snapshot with the new name for the directory */ public void renameSnapshot(final INodesInPath iip, final String snapshotRoot, - final String oldSnapshotName, final String newSnapshotName) + final String oldSnapshotName, final String newSnapshotName, long now) throws IOException { final INodeDirectory srcRoot = getSnapshottableRoot(iip); - srcRoot.renameSnapshot(snapshotRoot, oldSnapshotName, newSnapshotName); + srcRoot.renameSnapshot(snapshotRoot, oldSnapshotName, newSnapshotName, now); } public int getNumSnapshottableDirs() { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java index 5a5092c693..e78ce374f6 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestSnapshot.java @@ -19,6 +19,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -472,6 +473,23 @@ public void testSnapshotMtime() throws Exception { newSnapshotStatus.getModificationTime()); } + @Test(timeout = 60000) + public void testRenameSnapshotMtime() throws Exception { + Path dir = new Path("/dir"); + Path sub = new Path(dir, "sub"); + Path subFile = new Path(sub, "file"); + DFSTestUtil.createFile(hdfs, subFile, BLOCKSIZE, REPLICATION, seed); + + hdfs.allowSnapshot(dir); + Path snapshotPath = hdfs.createSnapshot(dir, "s1"); + FileStatus oldSnapshotStatus = hdfs.getFileStatus(snapshotPath); + hdfs.renameSnapshot(dir, "s1", "s2"); + Path snapshotRenamePath = new Path("/dir/.snapshot/s2"); + FileStatus newSnapshotStatus = hdfs.getFileStatus(snapshotRenamePath); + assertNotEquals(oldSnapshotStatus.getModificationTime(), + newSnapshotStatus.getModificationTime()); + } + /** * Prepare a list of modifications. A modification may be a file creation, * file deletion, or a modification operation such as appending to an existing diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored b/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/editsStored index 9f105fb88e296c6a59280d3b552479b4a537fe40..6b340fae5826b8f43b107567e288abc4224462cd 100644 GIT binary patch delta 48 zcmaEB`_*/directory_mkdir snapshot1 snapshot2 + 1512607197715 cab1aa2d-e08a-4d2f-8216-76e167eccd94 55