From d9f453500e74bc696cedce22d8a3e9af278e3765 Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Wed, 28 Jul 2021 10:33:41 +0800 Subject: [PATCH 1/3] HDFS-14529. SetTimes to throw FileNotFoundException if inode is not found. Change-Id: I6715b158b5ae3f198178d52cab9645a7b696fe1c --- .../hdfs/server/namenode/FSDirAttrOp.java | 13 ++++++------- .../hdfs/server/namenode/TestFSDirAttrOp.java | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java index a2d57cfa6537c..ac4744e490cc7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java @@ -124,11 +124,6 @@ static FileStatus setTimes( if (fsd.isPermissionEnabled()) { fsd.checkPathAccess(pc, iip, FsAction.WRITE); } - final INode inode = iip.getLastINode(); - if (inode == null) { - throw new FileNotFoundException("File/Directory " + iip.getPath() + - " does not exist."); - } boolean changed = unprotectedSetTimes(fsd, iip, mtime, atime, true); if (changed) { fsd.getEditLog().logTimes(iip.getPath(), mtime, atime); @@ -305,7 +300,7 @@ static boolean unprotectedSetOwner( static boolean setTimes( FSDirectory fsd, INodesInPath iip, long mtime, long atime, boolean force) - throws QuotaExceededException { + throws QuotaExceededException, FileNotFoundException { fsd.writeLock(); try { return unprotectedSetTimes(fsd, iip, mtime, atime, force); @@ -497,10 +492,14 @@ private static void setDirStoragePolicy( static boolean unprotectedSetTimes( FSDirectory fsd, INodesInPath iip, long mtime, long atime, boolean force) - throws QuotaExceededException { + throws QuotaExceededException, FileNotFoundException { assert fsd.hasWriteLock(); boolean status = false; INode inode = iip.getLastINode(); + if (inode == null) { + throw new FileNotFoundException("File/Directory " + iip.getPath() + + " does not exist."); + } int latest = iip.getLatestSnapshotId(); if (mtime >= 0) { inode = inode.setModificationTime(mtime, latest); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java index 3d391b0468761..c430681f43ec5 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java @@ -28,6 +28,8 @@ import org.junit.Test; import org.mockito.Mockito; +import java.io.FileNotFoundException; + import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.when; @@ -40,7 +42,8 @@ public class TestFSDirAttrOp { LoggerFactory.getLogger(TestFSDirAttrOp.class); private boolean unprotectedSetTimes(long atime, long atime0, long precision, - long mtime, boolean force) throws QuotaExceededException { + long mtime, boolean force) + throws QuotaExceededException, FileNotFoundException { FSNamesystem fsn = Mockito.mock(FSNamesystem.class); SnapshotManager ssMgr = Mockito.mock(SnapshotManager.class); FSDirectory fsd = Mockito.mock(FSDirectory.class); @@ -131,4 +134,16 @@ public void testUnprotectedSetTimes() throws Exception { assertTrue("SetTimes should update access time", unprotectedSetTimes(100, 0, 1000, 1, false)); } + + @Test(expected = FileNotFoundException.class) + public void testUnprotectedSetTimesFNFE() + throws QuotaExceededException, FileNotFoundException { + FSDirectory fsd = Mockito.mock(FSDirectory.class); + INodesInPath iip = Mockito.mock(INodesInPath.class); + + when(fsd.hasWriteLock()).thenReturn(Boolean.TRUE); + when(iip.getLastINode()).thenReturn(null); + + FSDirAttrOp.unprotectedSetTimes(fsd, iip, 0, 0, false); + } } From 9458f1506d31fb1e5158b962a06a5730409784f1 Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Wed, 28 Jul 2021 16:23:36 +0800 Subject: [PATCH 2/3] Remove unthrown exception Change-Id: I816a25b208bcb8c9605c68a1dd7deaee34e221d6 --- .../org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java | 4 ++-- .../apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java index ac4744e490cc7..db1baab66b3dc 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java @@ -300,7 +300,7 @@ static boolean unprotectedSetOwner( static boolean setTimes( FSDirectory fsd, INodesInPath iip, long mtime, long atime, boolean force) - throws QuotaExceededException, FileNotFoundException { + throws FileNotFoundException { fsd.writeLock(); try { return unprotectedSetTimes(fsd, iip, mtime, atime, force); @@ -492,7 +492,7 @@ private static void setDirStoragePolicy( static boolean unprotectedSetTimes( FSDirectory fsd, INodesInPath iip, long mtime, long atime, boolean force) - throws QuotaExceededException, FileNotFoundException { + throws FileNotFoundException { assert fsd.hasWriteLock(); boolean status = false; INode inode = iip.getLastINode(); diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java index c430681f43ec5..f283793cb7d7a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java @@ -43,7 +43,7 @@ public class TestFSDirAttrOp { private boolean unprotectedSetTimes(long atime, long atime0, long precision, long mtime, boolean force) - throws QuotaExceededException, FileNotFoundException { + throws FileNotFoundException { FSNamesystem fsn = Mockito.mock(FSNamesystem.class); SnapshotManager ssMgr = Mockito.mock(SnapshotManager.class); FSDirectory fsd = Mockito.mock(FSDirectory.class); @@ -137,7 +137,7 @@ public void testUnprotectedSetTimes() throws Exception { @Test(expected = FileNotFoundException.class) public void testUnprotectedSetTimesFNFE() - throws QuotaExceededException, FileNotFoundException { + throws FileNotFoundException { FSDirectory fsd = Mockito.mock(FSDirectory.class); INodesInPath iip = Mockito.mock(INodesInPath.class); From 458a11982967070f763025ffbe4b05b5e6f854eb Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Thu, 29 Jul 2021 11:07:20 +0800 Subject: [PATCH 3/3] Fix checkstyle Change-Id: I6a46aef7cf9ce81d1aebd1072fd45bb75cced3bb --- .../org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java | 1 - 1 file changed, 1 deletion(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java index f283793cb7d7a..df7ab3dd9e7bb 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java @@ -23,7 +23,6 @@ import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.fs.permission.PermissionStatus; import org.apache.hadoop.hdfs.DFSUtil; -import org.apache.hadoop.hdfs.protocol.QuotaExceededException; import org.apache.hadoop.hdfs.server.namenode.snapshot.SnapshotManager; import org.junit.Test; import org.mockito.Mockito;