HDFS-11258. File mtime change could not save to editlog. Contributed by Jimmy Xiang.
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 e19341c..c56d574 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
@@ -517,17 +517,13 @@
inode = inode.setModificationTime(mtime, latest);
status = true;
}
- if (atime != -1) {
- long inodeTime = inode.getAccessTime();
- // if the last access time update was within the last precision interval, then
- // no need to store access time
- if (atime <= inodeTime + fsd.getAccessTimePrecision() && !force) {
- status = false;
- } else {
- inode.setAccessTime(atime, latest);
- status = true;
- }
+ // if the last access time update was within the last precision interval,
+ // then no need to store access time
+ if (atime != -1 && (status || force
+ || atime > inode.getAccessTime() + fsd.getAccessTimePrecision())) {
+ inode.setAccessTime(atime, latest);
+ status = true;
}
return status;
}
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
new file mode 100644
index 0000000..c75e8ec
--- /dev/null
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSDirAttrOp.java
@@ -0,0 +1,80 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdfs.server.namenode;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.fs.UnresolvedLinkException;
+import org.apache.hadoop.hdfs.protocol.QuotaExceededException;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.when;
+
+/**
+ * Test {@link FSDirAttrOp}.
+ */
+public class TestFSDirAttrOp {
+ public static final Log LOG = LogFactory.getLog(TestFSDirAttrOp.class);
+
+ private boolean unprotectedSetTimes(long atime, long atime0, long precision,
+ long mtime, boolean force) throws QuotaExceededException, UnresolvedLinkException {
+ FSDirectory fsd = Mockito.mock(FSDirectory.class);
+ INodesInPath iip = Mockito.mock(INodesInPath.class);
+ INode inode = Mockito.mock(INode.class);
+
+ when(fsd.getAccessTimePrecision()).thenReturn(precision);
+ when(fsd.hasWriteLock()).thenReturn(Boolean.TRUE);
+ when(fsd.getINodesInPath(anyString(), anyBoolean())).thenReturn(iip);
+ when(iip.getLastINode()).thenReturn(inode);
+ when(iip.getLatestSnapshotId()).thenReturn(Mockito.anyInt());
+ when(inode.getAccessTime()).thenReturn(atime0);
+
+ return FSDirAttrOp.unprotectedSetTimes(fsd, "", mtime, atime, force);
+ }
+
+ @Test
+ public void testUnprotectedSetTimes() throws Exception {
+ // atime < access time + precision
+ assertFalse("SetTimes should not update access time"
+ + "because it's within the last precision interval",
+ unprotectedSetTimes(100, 0, 1000, -1, false));
+
+ // atime = access time + precision
+ assertFalse("SetTimes should not update access time"
+ + "because it's within the last precision interval",
+ unprotectedSetTimes(1000, 0, 1000, -1, false));
+
+ // atime > access time + precision
+ assertTrue("SetTimes should store access time",
+ unprotectedSetTimes(1011, 10, 1000, -1, false));
+
+ // atime < access time + precision, but force is set
+ assertTrue("SetTimes should store access time",
+ unprotectedSetTimes(100, 0, 1000, -1, true));
+
+ // atime < access time + precision, but mtime is set
+ assertTrue("SetTimes should store access time",
+ unprotectedSetTimes(100, 0, 1000, 1, false));
+ }
+}
