HADOOP-12045. Enable LocalFileSystem#setTimes to change atime. Contributed by Kazuho Fujii.

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -675,6 +675,9 @@ Release 2.8.0 - UNRELEASED
 
     HADOOP-12171. Shorten overly-long htrace span names for server (cmccabe)
 
+    HADOOP-12045. Enable LocalFileSystem#setTimes to change atime.
+    (Kazuho Fujii via cnauroth)
+
   OPTIMIZATIONS
 
     HADOOP-11785. Reduce the number of listStatus operation in distcp

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java

@@ -33,6 +33,10 @@ import java.io.OutputStream;
 import java.io.FileDescriptor;
 import java.net.URI;
 import java.nio.ByteBuffer;
+import java.nio.file.Files;
+import java.nio.file.attribute.BasicFileAttributes;
+import java.nio.file.attribute.BasicFileAttributeView;
+import java.nio.file.attribute.FileTime;
 import java.util.Arrays;
 import java.util.EnumSet;
 import java.util.StringTokenizer;
@@ -644,9 +648,14 @@ public class RawLocalFileSystem extends FileSystem {
       return !super.getOwner().isEmpty(); 
     }
     
-    DeprecatedRawLocalFileStatus(File f, long defaultBlockSize, FileSystem fs) {
+    DeprecatedRawLocalFileStatus(File f, long defaultBlockSize, FileSystem fs)
+      throws IOException {
       super(f.length(), f.isDirectory(), 1, defaultBlockSize,
-          f.lastModified(), new Path(f.getPath()).makeQualified(fs.getUri(),
+          f.lastModified(),
+          Files.readAttributes(f.toPath(),
+            BasicFileAttributes.class).lastAccessTime().toMillis(),
+          null, null, null,
+          new Path(f.getPath()).makeQualified(fs.getUri(),
             fs.getWorkingDirectory()));
     }
     
@@ -758,25 +767,20 @@ public class RawLocalFileSystem extends FileSystem {
   }
  
   /**
-   * Sets the {@link Path}'s last modified time <em>only</em> to the given
+   * Sets the {@link Path}'s last modified time and last access time to
-   * valid time.
+   * the given valid times.
    *
    * @param mtime the modification time to set (only if greater than zero).
-   * @param atime currently ignored.
+   * @param atime the access time to set (only if greater than zero).
-   * @throws IOException if setting the last modified time fails.
+   * @throws IOException if setting the times fails.
    */
   @Override
   public void setTimes(Path p, long mtime, long atime) throws IOException {
-    File f = pathToFile(p);
+    BasicFileAttributeView view = Files.getFileAttributeView(
-    if(mtime >= 0) {
+        pathToFile(p).toPath(), BasicFileAttributeView.class);
-      if(!f.setLastModified(mtime)) {
+    FileTime fmtime = (mtime >= 0) ? FileTime.fromMillis(mtime) : null;
-        throw new IOException(
+    FileTime fatime = (atime >= 0) ? FileTime.fromMillis(atime) : null;
-          "couldn't set last-modified time to " +
+    view.setTimes(fmtime, fatime, null);
-          mtime +
-          " for " +
-          f.getAbsolutePath());
-      }
-    }
   }
 
   @Override

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/SymlinkBaseTest.java

@@ -1386,19 +1386,48 @@ public abstract class SymlinkBaseTest {
   }
 
   @Test(timeout=10000)
-  /** setTimes affects the target not the link */
+  /** setTimes affects the target file not the link */
-  public void testSetTimes() throws IOException {
+  public void testSetTimesSymlinkToFile() throws IOException {
     Path file = new Path(testBaseDir1(), "file");
     Path link = new Path(testBaseDir1(), "linkToFile");
     createAndWriteFile(file);
     wrapper.createSymlink(file, link, false);
     long at = wrapper.getFileLinkStatus(link).getAccessTime();
-    wrapper.setTimes(link, 2L, 3L);
+    // the local file system may not support millisecond timestamps
-    // NB: local file systems don't implement setTimes
+    wrapper.setTimes(link, 2000L, 3000L);
-    if (!"file".equals(getScheme())) {
+    assertEquals(at, wrapper.getFileLinkStatus(link).getAccessTime());
-      assertEquals(at, wrapper.getFileLinkStatus(link).getAccessTime());
+    assertEquals(2000, wrapper.getFileStatus(file).getModificationTime());
-      assertEquals(3, wrapper.getFileStatus(file).getAccessTime());
+    assertEquals(3000, wrapper.getFileStatus(file).getAccessTime());
-      assertEquals(2, wrapper.getFileStatus(file).getModificationTime());
+  }
+
+  @Test(timeout=10000)
+  /** setTimes affects the target directory not the link */
+  public void testSetTimesSymlinkToDir() throws IOException {
+    Path dir = new Path(testBaseDir1(), "dir");
+    Path link = new Path(testBaseDir1(), "linkToDir");
+    wrapper.mkdir(dir, FileContext.DEFAULT_PERM, false);
+    wrapper.createSymlink(dir, link, false);
+    long at = wrapper.getFileLinkStatus(link).getAccessTime();
+    // the local file system may not support millisecond timestamps
+    wrapper.setTimes(link, 2000L, 3000L);
+    assertEquals(at, wrapper.getFileLinkStatus(link).getAccessTime());
+    assertEquals(2000, wrapper.getFileStatus(dir).getModificationTime());
+    assertEquals(3000, wrapper.getFileStatus(dir).getAccessTime());
+  }
+
+  @Test(timeout=10000)
+  /** setTimes does not affect the link even though target does not exist */
+  public void testSetTimesDanglingLink() throws IOException {
+    Path file = new Path("/noSuchFile");
+    Path link = new Path(testBaseDir1()+"/link");
+    wrapper.createSymlink(file, link, false);
+    long at = wrapper.getFileLinkStatus(link).getAccessTime();
+    try {
+      wrapper.setTimes(link, 2000L, 3000L);
+      fail("set times to non-existant file");
+    } catch (IOException e) {
+      // Expected
     }
+    assertEquals(at, wrapper.getFileLinkStatus(link).getAccessTime());
   }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystem.java

@@ -378,7 +378,14 @@ public class TestLocalFileSystem {
     assertTrue(dataFileFound);
     assertTrue(checksumFileFound);
   }
-  
+
+  private void checkTimesStatus(Path path,
+    long expectedModTime, long expectedAccTime) throws IOException {
+    FileStatus status = fileSys.getFileStatus(path);
+    assertEquals(expectedModTime, status.getModificationTime());
+    assertEquals(expectedAccTime, status.getAccessTime());
+  }
+
   @Test(timeout = 1000)
   public void testSetTimes() throws Exception {
     Path path = new Path(TEST_ROOT_DIR, "set-times");
@@ -387,15 +394,24 @@ public class TestLocalFileSystem {
     // test only to the nearest second, as the raw FS may not
     // support millisecond timestamps
     long newModTime = 12345000;
+    long newAccTime = 23456000;
 
     FileStatus status = fileSys.getFileStatus(path);
     assertTrue("check we're actually changing something", newModTime != status.getModificationTime());
-    long accessTime = status.getAccessTime();
+    assertTrue("check we're actually changing something", newAccTime != status.getAccessTime());
+
+    fileSys.setTimes(path, newModTime, newAccTime);
+    checkTimesStatus(path, newModTime, newAccTime);
+
+    newModTime = 34567000;
 
     fileSys.setTimes(path, newModTime, -1);
-    status = fileSys.getFileStatus(path);
+    checkTimesStatus(path, newModTime, newAccTime);
-    assertEquals(newModTime, status.getModificationTime());
+
-    assertEquals(accessTime, status.getAccessTime());
+    newAccTime = 45678000;
+
+    fileSys.setTimes(path, -1, newAccTime);
+    checkTimesStatus(path, newModTime, newAccTime);
   }
 
   /**

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestSymlinkLocalFS.java

@@ -231,4 +231,22 @@ abstract public class TestSymlinkLocalFS extends SymlinkBaseTest {
       // Expected.
     }
   }
+
+  @Override
+  public void testSetTimesSymlinkToFile() throws IOException {
+    assumeTrue(!Path.WINDOWS);
+    super.testSetTimesSymlinkToFile();
+  }
+
+  @Override
+  public void testSetTimesSymlinkToDir() throws IOException {
+    assumeTrue(!Path.WINDOWS);
+    super.testSetTimesSymlinkToDir();
+  }
+
+  @Override
+  public void testSetTimesDanglingLink() throws IOException {
+    assumeTrue(!Path.WINDOWS);
+    super.testSetTimesDanglingLink();
+  }
 }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCopyPreserveFlag.java

@@ -18,12 +18,13 @@
 package org.apache.hadoop.fs.shell;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertNotEquals;
 
 import java.io.IOException;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.LocalFileSystem;
 import org.apache.hadoop.fs.Path;
@@ -38,8 +39,12 @@ import org.junit.Test;
 
 public class TestCopyPreserveFlag {
   private static final int MODIFICATION_TIME = 12345000;
-  private static final Path FROM = new Path("d1", "f1");
+  private static final int ACCESS_TIME = 23456000;
-  private static final Path TO = new Path("d2", "f2");
+  private static final Path DIR_FROM = new Path("d0");
+  private static final Path DIR_TO1 = new Path("d1");
+  private static final Path DIR_TO2 = new Path("d2");
+  private static final Path FROM = new Path(DIR_FROM, "f0");
+  private static final Path TO = new Path(DIR_TO1, "f1");
   private static final FsPermission PERMISSIONS = new FsPermission(
     FsAction.ALL,
     FsAction.EXECUTE,
@@ -62,8 +67,8 @@ public class TestCopyPreserveFlag {
 
     FileSystem.setDefaultUri(conf, fs.getUri());
     fs.setWorkingDirectory(testDir);
-    fs.mkdirs(new Path("d1"));
+    fs.mkdirs(DIR_FROM);
-    fs.mkdirs(new Path("d2"));
+    fs.mkdirs(DIR_TO1);
     fs.createNewFile(FROM);
 
     FSDataOutputStream output = fs.create(FROM, true);
@@ -72,10 +77,10 @@ public class TestCopyPreserveFlag {
         output.writeChar('\n');
     }
     output.close();
-    fs.setTimes(FROM, MODIFICATION_TIME, 0);
+    fs.setTimes(FROM, MODIFICATION_TIME, ACCESS_TIME);
     fs.setPermission(FROM, PERMISSIONS);
-    fs.setTimes(new Path("d1"), MODIFICATION_TIME, 0);
+    fs.setTimes(DIR_FROM, MODIFICATION_TIME, ACCESS_TIME);
-    fs.setPermission(new Path("d1"), PERMISSIONS);
+    fs.setPermission(DIR_FROM, PERMISSIONS);
   }
 
   @After
@@ -84,14 +89,18 @@ public class TestCopyPreserveFlag {
     fs.close();
   }
 
-  private void assertAttributesPreserved() throws IOException {
+  private void assertAttributesPreserved(Path to) throws IOException {
-    assertEquals(MODIFICATION_TIME, fs.getFileStatus(TO).getModificationTime());
+    FileStatus status = fs.getFileStatus(to);
-    assertEquals(PERMISSIONS, fs.getFileStatus(TO).getPermission());
+    assertEquals(MODIFICATION_TIME, status.getModificationTime());
+    assertEquals(ACCESS_TIME, status.getAccessTime());
+    assertEquals(PERMISSIONS, status.getPermission());
   }
 
-  private void assertAttributesChanged() throws IOException {
+  private void assertAttributesChanged(Path to) throws IOException {
-      assertTrue(MODIFICATION_TIME != fs.getFileStatus(TO).getModificationTime());
+    FileStatus status = fs.getFileStatus(to);
-      assertTrue(!PERMISSIONS.equals(fs.getFileStatus(TO).getPermission()));
+    assertNotEquals(MODIFICATION_TIME, status.getModificationTime());
+    assertNotEquals(ACCESS_TIME, status.getAccessTime());
+    assertNotEquals(PERMISSIONS, status.getPermission());
   }
 
   private void run(CommandWithDestination cmd, String... args) {
@@ -102,54 +111,48 @@ public class TestCopyPreserveFlag {
   @Test(timeout = 10000)
   public void testPutWithP() throws Exception {
     run(new Put(), "-p", FROM.toString(), TO.toString());
-    assertAttributesPreserved();
+    assertAttributesPreserved(TO);
   }
 
   @Test(timeout = 10000)
   public void testPutWithoutP() throws Exception {
     run(new Put(), FROM.toString(), TO.toString());
-    assertAttributesChanged();
+    assertAttributesChanged(TO);
   }
 
   @Test(timeout = 10000)
   public void testGetWithP() throws Exception {
     run(new Get(), "-p", FROM.toString(), TO.toString());
-    assertAttributesPreserved();
+    assertAttributesPreserved(TO);
   }
 
   @Test(timeout = 10000)
   public void testGetWithoutP() throws Exception {
     run(new Get(), FROM.toString(), TO.toString());
-    assertAttributesChanged();
+    assertAttributesChanged(TO);
   }
 
   @Test(timeout = 10000)
   public void testCpWithP() throws Exception {
       run(new Cp(), "-p", FROM.toString(), TO.toString());
-      assertAttributesPreserved();
+      assertAttributesPreserved(TO);
   }
 
   @Test(timeout = 10000)
   public void testCpWithoutP() throws Exception {
       run(new Cp(), FROM.toString(), TO.toString());
-      assertAttributesChanged();
+      assertAttributesChanged(TO);
   }
 
   @Test(timeout = 10000)
   public void testDirectoryCpWithP() throws Exception {
-    run(new Cp(), "-p", "d1", "d3");
+    run(new Cp(), "-p", DIR_FROM.toString(), DIR_TO2.toString());
-    assertEquals(fs.getFileStatus(new Path("d1")).getModificationTime(),
+    assertAttributesPreserved(DIR_TO2);
-        fs.getFileStatus(new Path("d3")).getModificationTime());
-    assertEquals(fs.getFileStatus(new Path("d1")).getPermission(),
-        fs.getFileStatus(new Path("d3")).getPermission());
   }
 
   @Test(timeout = 10000)
   public void testDirectoryCpWithoutP() throws Exception {
-    run(new Cp(), "d1", "d4");
+    run(new Cp(), DIR_FROM.toString(), DIR_TO2.toString());
-    assertTrue(fs.getFileStatus(new Path("d1")).getModificationTime() !=
+    assertAttributesChanged(DIR_TO2);
-        fs.getFileStatus(new Path("d4")).getModificationTime());
-    assertTrue(!fs.getFileStatus(new Path("d1")).getPermission()
-        .equals(fs.getFileStatus(new Path("d4")).getPermission()));
   }
 }