hadoop-common-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sur...@apache.org
Subject svn commit: r1417935 - in /hadoop/common/branches/branch-1-win: ./ src/contrib/streaming/ src/core/org/apache/hadoop/fs/ src/core/org/apache/hadoop/io/nativeio/ src/core/org/apache/hadoop/util/ src/native/src/org/apache/hadoop/io/nativeio/ src/test/org...
Date Thu, 06 Dec 2012 15:22:06 GMT
Author: suresh
Date: Thu Dec  6 15:22:04 2012
New Revision: 1417935

URL: http://svn.apache.org/viewvc?rev=1417935&view=rev
Log:
HADOOP-9061. Java6+Windows does not work well with symlinks. Contributed by Ivan Mitic.

Modified:
    hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt
    hadoop/common/branches/branch-1-win/src/contrib/streaming/ivy.xml
    hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FileUtil.java
    hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java
    hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/io/nativeio/NativeIO.java
    hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/Shell.java
    hadoop/common/branches/branch-1-win/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c
    hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/filecache/TestMRWithDistributedCache.java
    hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFileUtil.java

Modified: hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt?rev=1417935&r1=1417934&r2=1417935&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt (original)
+++ hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt Thu Dec  6 15:22:04 2012
@@ -237,3 +237,6 @@ Branch-hadoop-1-win - unreleased
 
     HADOOP-9102. winutils task isAlive does not return a non-zero exit code if
     the requested task is not alive. (Chris Nauroth via suresh)
+
+    HADOOP-9061. Java6+Windows does not work well with symlinks.
+    (Ivan Mitic via suresh)

Modified: hadoop/common/branches/branch-1-win/src/contrib/streaming/ivy.xml
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/contrib/streaming/ivy.xml?rev=1417935&r1=1417934&r2=1417935&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/contrib/streaming/ivy.xml (original)
+++ hadoop/common/branches/branch-1-win/src/contrib/streaming/ivy.xml Thu Dec  6 15:22:04
2012
@@ -28,6 +28,10 @@
       name="commons-cli"
       rev="${commons-cli.version}"
       conf="common->default"/>
+    <dependency org="commons-io"
+      name="commons-io"
+      rev="${commons-io.version}"
+      conf="common->default"/>
     <dependency org="commons-logging"
       name="commons-logging"
       rev="${commons-logging.version}"

Modified: hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FileUtil.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FileUtil.java?rev=1417935&r1=1417934&r2=1417935&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FileUtil.java (original)
+++ hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FileUtil.java Thu Dec
 6 15:22:04 2012
@@ -623,11 +623,34 @@ public class FileUtil {
    */
   public static int symLink(String target, String linkname) throws IOException{
     // Run the input paths through Java's File so that they are converted to the
-    // native OS form. FIXME: Long term fix is to expose symLink API that
-    // accepts File instead of String, as symlinks can only be created on the
-    // local FS.
-    String[] cmd = Shell.getSymlinkCommand(new File(target).getPath(),
-        new File(linkname).getPath());
+    // native OS form
+    File targetFile = new File(target);
+    File linkFile = new File(linkname);
+
+    // If not on Java7+, copy a file instead of creating a symlink since
+    // Java6 has close to no support for symlinks on Windows. Specifically
+    // File#length and File#renameTo do not work as expected.
+    // (see HADOOP-9061 for additional details)
+    // We still create symlinks for directories, since the scenario in this
+    // case is different. The directory content could change in which
+    // case the symlink loses its purpose (for example task attempt log folder
+    // is symlinked under userlogs and userlogs are generated afterwards).
+    if (Shell.WINDOWS && !Shell.isJava7OrAbove() && targetFile.isFile())
{
+      try {
+        LOG.info("FileUtil#symlink: On Java6, copying file instead "
+            + linkname + " -> " + target);
+        org.apache.commons.io.FileUtils.copyFile(targetFile, linkFile);
+      } catch (IOException ex) {
+        LOG.warn("FileUtil#symlink failed to copy the file with error: "
+            + ex.getMessage());
+        // Exit with non-zero exit code
+        return 1;
+      }
+      return 0;
+    }
+
+    String[] cmd = Shell.getSymlinkCommand(targetFile.getPath(),
+        linkFile.getPath());
     ShellCommandExecutor shExec = new ShellCommandExecutor(cmd);
     try {
       shExec.execute();
@@ -908,48 +931,4 @@ public class FileUtil {
     jarStream.close();
     return jarFile;
   }
-
-  /** Returns the file length. In case of a symbolic link, follows the link
-   *  and returns the target file length. API is used to provide
-   *  transparent behavior between Unix and Windows since on Windows
-   *  {@link File#length()} does not follow symbolic links.
-   * @param file file to extract the length for
-   *
-   * @throws IOException if an error occurred.
-   */
-  public static long getLengthFollowSymlink(File file) {
-    return getLengthFollowSymlink(file, false);
-  }
-
-  /** Package level API used for unit-testing only. */
-  static long getLengthFollowSymlink(File file, boolean disableNativeIO) {
-    if (!Shell.WINDOWS) {
-      return file.length();
-    } else {
-      try {
-        // FIXME: Below logic is not needed On Java 7 under Windows since
-        // File#length returns the correct value.
-        if (!disableNativeIO && NativeIO.isAvailable()) {
-          // Use Windows native API for extracting the file length if NativeIO
-          // is available.
-          return NativeIO.Windows.getLengthFollowSymlink(
-            file.getCanonicalPath());
-        } else {
-          // If NativeIO is not available, we have to provide the fallback
-          // mechanism since downstream Hadoop projects do not want
-          // to worry about adding hadoop.dll to the java library path.
-
-          // Extract the target link size via winutils
-          String output = FileUtil.execCommand(
-            file, new String[] { Shell.WINUTILS, "ls", "-L", "-F" });
-          // Output tokens are separated with a pipe character
-          String[] args = output.split("\\|");
-          return Long.parseLong(args[4]);
-        }
-      } catch (IOException ex) {
-        // In case of an error return zero to be consistent with File#length
-        return 0;
-      }
-    }
-  }
 }

Modified: hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java?rev=1417935&r1=1417934&r2=1417935&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java
(original)
+++ hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java
Thu Dec  6 15:22:04 2012
@@ -435,11 +435,9 @@ public class RawLocalFileSystem extends 
       return !super.getOwner().equals(""); 
     }
     
-    RawLocalFileStatus(File f, long defaultBlockSize, FileSystem fs) {
-      // Extract File#length thru FileUtil#getLengthFollowSymlink to provide
-      // the same behavior for symbolic links on different platforms. 
-      super(FileUtil.getLengthFollowSymlink(f), f.isDirectory(), 1, defaultBlockSize,
-            f.lastModified(), new Path(f.getPath()).makeQualified(fs));
+    RawLocalFileStatus(File f, long defaultBlockSize, FileSystem fs) { 
+      super(f.length(), f.isDirectory(), 1, defaultBlockSize,
+          f.lastModified(), new Path(f.getPath()).makeQualified(fs));
     }
     
     @Override

Modified: hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/io/nativeio/NativeIO.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/io/nativeio/NativeIO.java?rev=1417935&r1=1417934&r2=1417935&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/io/nativeio/NativeIO.java
(original)
+++ hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/io/nativeio/NativeIO.java
Thu Dec  6 15:22:04 2012
@@ -177,10 +177,6 @@ public class NativeIO {
     /** Windows only methods used for getOwner() implementation */
     private static native String getOwner(FileDescriptor fd) throws IOException;
 
-    /** Windows only method used for getting the file length */
-    public static native long getLengthFollowSymlink(
-        String path) throws IOException;
-
     static {
       if (NativeCodeLoader.isNativeCodeLoaded()) {
         try {

Modified: hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/Shell.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/Shell.java?rev=1417935&r1=1417934&r2=1417935&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/Shell.java (original)
+++ hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/Shell.java Thu Dec
 6 15:22:04 2012
@@ -41,6 +41,13 @@ abstract public class Shell {
   
   public static final Log LOG = LogFactory.getLog(Shell.class);
 
+  private static boolean IS_JAVA7_OR_ABOVE =
+      System.getProperty("java.version").substring(0, 3).compareTo("1.7") >= 0;
+
+  public static boolean isJava7OrAbove() {
+    return IS_JAVA7_OR_ABOVE;
+  }
+
   /** a Windows utility to emulate Unix commands */
   public static final String WINUTILS = System.getenv("HADOOP_HOME")
                                         + "\\bin\\winutils";

Modified: hadoop/common/branches/branch-1-win/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c?rev=1417935&r1=1417934&r2=1417935&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c
(original)
+++ hadoop/common/branches/branch-1-win/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c
Thu Dec  6 15:22:04 2012
@@ -604,42 +604,6 @@ cleanup:
 #endif
 }
 
-/*
- * Class:     org_apache_hadoop_io_nativeio_NativeIO_Windows
- * Method:    getLengthFollowSymlink
- * Signature: (Ljava/lang/String;)J
- */
-JNIEXPORT jlong JNICALL Java_org_apache_hadoop_io_nativeio_NativeIO_00024Windows_getLengthFollowSymlink
-  (JNIEnv *env, jclass clazz, jstring j_path)
-{
-#ifdef UNIX
-  THROW(env, "java/io/IOException",
-    "The function getLengthFollowSymlink(String) is not supported on Unix");
-  return 0;
-#endif
-
-#ifdef WINDOWS
-  DWORD dwRtnCode = ERROR_SUCCESS;
-  BY_HANDLE_FILE_INFORMATION fileInfo = { 0 };
-  LARGE_INTEGER fileSize = { 0 };
-
-  const wchar_t *path = (const wchar_t*) (*env)->GetStringChars(env, j_path, NULL);
-  if (path == NULL) return 0; // JVM throws Exception for us
-
-  dwRtnCode = GetFileInformationByName(path, TRUE, &fileInfo);
-  if (dwRtnCode != ERROR_SUCCESS) {
-    throw_ioe(env, dwRtnCode);
-  }
-
-  (*env)->ReleaseStringChars(env, j_path, path);
-
-  fileSize.HighPart = fileInfo.nFileSizeHigh;
-  fileSize.LowPart = fileInfo.nFileSizeLow;
-
-  return (jlong)(fileSize.QuadPart);
-#endif
-}
-
 /**
  * vim: sw=2: ts=2: et:
  */

Modified: hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/filecache/TestMRWithDistributedCache.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/filecache/TestMRWithDistributedCache.java?rev=1417935&r1=1417934&r2=1417935&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/filecache/TestMRWithDistributedCache.java
(original)
+++ hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/filecache/TestMRWithDistributedCache.java
Thu Dec  6 15:22:04 2012
@@ -119,10 +119,7 @@ public class TestMRWithDistributedCache 
           context.getConfiguration().get("mapred.job.tracker"))) {
         File symlinkFile = new File("distributed.first.symlink");
         TestCase.assertTrue(symlinkFile.exists());
-        // Java 6 File#length returns zero for symbolic links on Windows
-        // FIXME: File#length for symbolic links may be due to change in Java 7
-        int expectedValue = Shell.WINDOWS ? 0 : 1;
-        TestCase.assertEquals(expectedValue, symlinkFile.length());
+        TestCase.assertEquals(1, symlinkFile.length());
       }
     }
   }

Modified: hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFileUtil.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFileUtil.java?rev=1417935&r1=1417934&r2=1417935&view=diff
==============================================================================
--- hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFileUtil.java (original)
+++ hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFileUtil.java Thu
Dec  6 15:22:04 2012
@@ -351,7 +351,7 @@ public class TestFileUtil {
 
     //ensure that symlink length is correctly reported by Java
     Assert.assertEquals(data.length, file.length());
-    Assert.assertEquals(Shell.WINDOWS ? 0 : data.length, link.length());
+    Assert.assertEquals(data.length, link.length());
 
     //ensure that we can read from link.
     FileInputStream in = new FileInputStream(link);
@@ -364,10 +364,66 @@ public class TestFileUtil {
   }
 
   /**
-   * Test the value of File's length extracted using FileUtil.
+   * Test that rename on a symlink works as expected.
    */
   @Test
-  public void testGetLengthFollowSymlink() throws Exception {
+  public void testSymlinkRenameTo() throws Exception {
+    Assert.assertFalse(del.exists());
+    del.mkdirs();
+
+    File file = new File(del, FILE);
+    file.createNewFile();
+    File link = new File(del, "_link");
+
+    // create the symlink
+    FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath());
+
+    Assert.assertTrue(file.exists());
+    Assert.assertTrue(link.exists());
+
+    File link2 = new File(del, "_link2");
+
+    // Rename the symlink
+    Assert.assertTrue(link.renameTo(link2));
+
+    // Make sure the file still exists
+    // (NOTE: this would fail on Java6 on Windows if we didn't
+    // copy the file in FileUtil#symlink)
+    Assert.assertTrue(file.exists());
+
+    Assert.assertTrue(link2.exists());
+    Assert.assertFalse(link.exists());
+  }
+
+  /**
+   * Test that deletion of a symlink works as expected.
+   */
+  @Test
+  public void testSymlinkDelete() throws Exception {
+    Assert.assertFalse(del.exists());
+    del.mkdirs();
+
+    File file = new File(del, FILE);
+    file.createNewFile();
+    File link = new File(del, "_link");
+
+    // create the symlink
+    FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath());
+
+    Assert.assertTrue(file.exists());
+    Assert.assertTrue(link.exists());
+
+    // make sure that deleting a symlink works properly
+    Assert.assertTrue(link.delete());
+    Assert.assertFalse(link.exists());
+    Assert.assertTrue(file.exists());
+  }
+
+  /**
+   * Test that length on a symlink works as expected.
+   */
+  @Test
+  public void testSymlinkLength() throws Exception {
     Assert.assertFalse(del.exists());
     del.mkdirs();
 
@@ -381,37 +437,30 @@ public class TestFileUtil {
     os.write(data);
     os.close();
 
-    // ensure that getLengthFollowSymlink returns zero if a file
-    // does not exist
-    Assert.assertEquals(0, FileUtil.getLengthFollowSymlink(link));
+    Assert.assertEquals(0, link.length());
 
     // create the symlink
     FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath());
 
-    // ensure that getLengthFollowSymlink returns the target file and link size
-    Assert.assertEquals(data.length, FileUtil.getLengthFollowSymlink(file));
-    Assert.assertEquals(data.length, FileUtil.getLengthFollowSymlink(link));
-
-    // ensure that getLengthFollowSymlink returns the target file and link
-    // size when NativeIO is not used (tests the fallback functionality
-    // on Windows)
-    Assert.assertEquals(
-      data.length, FileUtil.getLengthFollowSymlink(file, true));
-    Assert.assertEquals(
-      data.length, FileUtil.getLengthFollowSymlink(link, true));
+    // ensure that File#length returns the target file and link size
+    Assert.assertEquals(data.length, file.length());
+    Assert.assertEquals(data.length, link.length());
 
-    // Make sure that files can be deleted (no remaining open handles)
     file.delete();
     Assert.assertFalse(file.exists());
 
-    // Link size should be zero when it's pointing to nothing
-    Assert.assertEquals(0, FileUtil.getLengthFollowSymlink(link));
+    if (Shell.WINDOWS && !Shell.isJava7OrAbove()) {
+      // On Java6 on Windows, we copied the file
+      Assert.assertEquals(data.length, link.length());
+    } else {
+      // Otherwise, the target file size is zero
+      Assert.assertEquals(0, link.length());
+    }
 
     link.delete();
     Assert.assertFalse(link.exists());
-
   }
-  
+
   private void doUntarAndVerify(File tarFile, File untarDir) 
                                  throws IOException {
     if (untarDir.exists() && !FileUtil.fullyDelete(untarDir)) {



Mime
View raw message