hadoop-common-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dhr...@apache.org
Subject svn commit: r719783 - in /hadoop/core/branches/branch-0.18: CHANGES.txt src/c++/libhdfs/hdfs.c src/contrib/fuse-dfs/src/fuse_dfs.c src/contrib/fuse-dfs/src/test/TestFuseDFS.java
Date Sat, 22 Nov 2008 01:27:30 GMT
Author: dhruba
Date: Fri Nov 21 17:27:30 2008
New Revision: 719783

URL: http://svn.apache.org/viewvc?rev=719783&view=rev
Log:
HADOOP-4616. Fuse-dfs can handle bad values from FileSystem.read call.
(Pete Wyckoff via dhruba)


Modified:
    hadoop/core/branches/branch-0.18/CHANGES.txt
    hadoop/core/branches/branch-0.18/src/c++/libhdfs/hdfs.c
    hadoop/core/branches/branch-0.18/src/contrib/fuse-dfs/src/fuse_dfs.c
    hadoop/core/branches/branch-0.18/src/contrib/fuse-dfs/src/test/TestFuseDFS.java

Modified: hadoop/core/branches/branch-0.18/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/core/branches/branch-0.18/CHANGES.txt?rev=719783&r1=719782&r2=719783&view=diff
==============================================================================
--- hadoop/core/branches/branch-0.18/CHANGES.txt (original)
+++ hadoop/core/branches/branch-0.18/CHANGES.txt Fri Nov 21 17:27:30 2008
@@ -34,6 +34,9 @@
     HADOOP-4647. NamenodeFsck should close the DFSClient it has created.
     (szetszwo)
 
+    HADOOP-4616. Fuse-dfs can handle bad values from FileSystem.read call.
+    (Pete Wyckoff via dhruba)
+
 Release 0.18.2 - 2008-11-03
 
   BUG FIXES

Modified: hadoop/core/branches/branch-0.18/src/c++/libhdfs/hdfs.c
URL: http://svn.apache.org/viewvc/hadoop/core/branches/branch-0.18/src/c%2B%2B/libhdfs/hdfs.c?rev=719783&r1=719782&r2=719783&view=diff
==============================================================================
--- hadoop/core/branches/branch-0.18/src/c++/libhdfs/hdfs.c (original)
+++ hadoop/core/branches/branch-0.18/src/c++/libhdfs/hdfs.c Fri Nov 21 17:27:30 2008
@@ -484,6 +484,9 @@
         if (noReadBytes > 0) {
             (*env)->GetByteArrayRegion(env, jbRarray, 0, noReadBytes, buffer);
         }  else {
+            if (noReadBytes == 0 || noReadBytes < -1) {
+              fprintf(stderr, "WARN: FSDataInputStream.read returned invalid return code
- libhdfs returning EOF, i.e., 0: %d\n", noReadBytes);
+            }
             //This is a valid case: there aren't any bytes left to read!
             noReadBytes = 0;
         }
@@ -541,6 +544,9 @@
         if (noReadBytes > 0) {
             (*env)->GetByteArrayRegion(env, jbRarray, 0, noReadBytes, buffer);
         }  else {
+            if (noReadBytes == 0 || noReadBytes < -1) {
+              fprintf(stderr, "WARN: FSDataInputStream.read returned invalid return code
- libhdfs returning EOF, i.e., 0: %d\n", noReadBytes);
+            }
             //This is a valid case: there aren't any bytes left to read!
             noReadBytes = 0;
         }

Modified: hadoop/core/branches/branch-0.18/src/contrib/fuse-dfs/src/fuse_dfs.c
URL: http://svn.apache.org/viewvc/hadoop/core/branches/branch-0.18/src/contrib/fuse-dfs/src/fuse_dfs.c?rev=719783&r1=719782&r2=719783&view=diff
==============================================================================
--- hadoop/core/branches/branch-0.18/src/contrib/fuse-dfs/src/fuse_dfs.c (original)
+++ hadoop/core/branches/branch-0.18/src/contrib/fuse-dfs/src/fuse_dfs.c Fri Nov 21 17:27:30
2008
@@ -840,17 +840,26 @@
   assert(fh->fs != NULL);
   assert(fh->hdfsFH != NULL);
 
+  // special case this as simplifies the rest of the logic to know the caller wanted >
0 bytes
+  if (size == 0)
+    return 0;
+
+  // If size is bigger than the read buffer, then just read right into the user supplied
buffer
   if (size >= dfs->rdbuffer_size) {
     int num_read;
-    int total_read = 0;
+    size_t total_read = 0;
     while (size - total_read > 0 && (num_read = hdfsPread(fh->fs, fh->hdfsFH,
offset + total_read, buf + total_read, size - total_read)) > 0) {
       total_read += num_read;
     }
+    // if there was an error before satisfying the current read, this logic declares it an
error
+    // and does not try to return any of the bytes read. Don't think it matters, so the code
+    // is just being conservative.
+    if (total_read < size && num_read < 0) {
+      total_read = -EIO;
+    }
     return total_read;
   }
 
-  assert(fh->bufferSize >= 0);
-
   //
   // Critical section - protect from multiple reads in different threads accessing the read
buffer
   // (no returns until end)
@@ -872,7 +881,7 @@
       offset + size > fh->buffersStartOffset + fh->bufferSize) 
     {
       // Read into the buffer from DFS
-      size_t num_read;
+      int num_read = 0;
       size_t total_read = 0;
 
       while (dfs->rdbuffer_size  - total_read > 0 && 
@@ -880,21 +889,31 @@
         total_read += num_read;
       }
 
-      if (num_read < 0) {
+      // if there was an error before satisfying the current read, this logic declares it
an error
+      // and does not try to return any of the bytes read. Don't think it matters, so the
code
+      // is just being conservative.
+      if (total_read < size && num_read < 0) {
         // invalidate the buffer 
         fh->bufferSize = 0; 
         syslog(LOG_ERR, "Read error - pread failed for %s with return code %d %s:%d", path,
(int)num_read, __FILE__, __LINE__);
         ret = -EIO;
       } else {
+        // Either EOF, all read or read beyond size, but then there was an error
         fh->bufferSize = total_read;
         fh->buffersStartOffset = offset;
 
         if (dfs->rdbuffer_size - total_read > 0) {
+          // assert(num_read == 0); this should be true since if num_read < 0 handled
above.
           isEOF = 1;
         }
       }
     }
-  if (ret == 0) {
+
+  //
+  // NOTE on EOF, fh->bufferSize == 0 and ret = 0 ,so the logic for copying data into
the caller's buffer is bypassed, and
+  //  the code returns 0 as required
+  //
+  if (ret == 0 && fh->bufferSize > 0) {
 
     assert(offset >= fh->buffersStartOffset);
     assert(fh->buf);
@@ -911,8 +930,6 @@
     
     memcpy(buf, offsetPtr, amount);
 
-    // fuse requires the below and the code should guarantee this assertion
-    assert(amount == size || isEOF);
     ret = amount;
   }
 
@@ -921,6 +938,13 @@
   //
   pthread_mutex_unlock(&fh->mutex);
 
+  // fuse requires the below and the code should guarantee this assertion
+  // 3 cases on return:
+  //   1. entire read satisfied
+  //   2. partial read and isEOF - including 0 size read
+  //   3. error 
+  assert(ret == size || isEOF || ret < 0);
+
   return ret;
 }
 
@@ -1469,8 +1493,9 @@
 
     if (fh->buf != NULL) {
       free(fh->buf);
-      pthread_mutex_destroy(&fh->mutex);
     }
+    // this is always created and initialized, so always destroy it. (see dfs_open)
+      pthread_mutex_destroy(&fh->mutex);
 
     free(fh);
 

Modified: hadoop/core/branches/branch-0.18/src/contrib/fuse-dfs/src/test/TestFuseDFS.java
URL: http://svn.apache.org/viewvc/hadoop/core/branches/branch-0.18/src/contrib/fuse-dfs/src/test/TestFuseDFS.java?rev=719783&r1=719782&r2=719783&view=diff
==============================================================================
--- hadoop/core/branches/branch-0.18/src/contrib/fuse-dfs/src/test/TestFuseDFS.java (original)
+++ hadoop/core/branches/branch-0.18/src/contrib/fuse-dfs/src/test/TestFuseDFS.java Fri Nov
21 17:27:30 2008
@@ -432,10 +432,15 @@
       FileStatus foo = fileSys.getFileStatus(myPath);
       assertTrue(foo.getLen() >= 9 * 1024);
 
+
       {
         // cat the file
         DataInputStream is = new DataInputStream(new FileInputStream(mpoint + "/test/hello.reads"));
         byte buf [] = new byte[4096];
+        // test reading 0 length
+        assertTrue(is.read(buf, 0, 0) == 0);
+
+        // test real reads
         assertTrue(is.read(buf, 0, 1024) == 1024);
         assertTrue(is.read(buf, 0, 4096) == 4096);
         assertTrue(is.read(buf, 0, 4096) == 4096);
@@ -458,6 +463,21 @@
           assertTrue(read >= 9 * 1024);
         }
       }
+
+      // check reading an empty file for EOF
+      {
+        // create the file
+        myPath = new Path("/test/hello.reads2");
+        s = fileSys.create(myPath);
+        s.close();
+
+        FSDataInputStream fs = fileSys.open(myPath);
+        assertEquals(-1,  fs.read());
+
+        FileInputStream f = new FileInputStream(mpoint + "/test/hello.reads2");
+        assertEquals(-1, f.read());
+      }
+
     } catch(Exception e) {
       e.printStackTrace();
     } finally {
@@ -469,7 +489,6 @@
    * Use filesys to create the hello world! file and then cat it and see its contents are
correct.
    */
   public void testCat() throws IOException,InterruptedException  {
-    if(true) return;
     try {
       // First create a new directory with mkdirs
       Runtime r = Runtime.getRuntime();



Mime
View raw message