You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by su...@apache.org on 2012/12/06 16:22:06 UTC
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...
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)) {