Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.20.2
    • Fix Version/s: 0.20.3, 0.22.0
    • Component/s: libhdfs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      In hdfsOpenFile in libhdfs invokeMethod needs to cast the block size argument to a jlong so a full 8 bytes are passed (rather than 4 plus some garbage which causes writes to fail due to a bogus block size).

      1. hdfs727.patch
        1 kB
        Eli Collins
      2. hdfs727.patch
        2 kB
        Eli Collins

        Issue Links

          Activity

          Eli Collins created issue -
          Eli Collins made changes -
          Field Original Value New Value
          Link This issue is blocked by HDFS-712 [ HDFS-712 ]
          Hide
          Eli Collins added a comment -

          This patch fixes the above bug, nukes an unused define, and makes a minor fix to the declaration of classname in constructNewArrayString (which running with -Xcheck:jni discovered).

          Show
          Eli Collins added a comment - This patch fixes the above bug, nukes an unused define, and makes a minor fix to the declaration of classname in constructNewArrayString (which running with -Xcheck:jni discovered).
          Eli Collins made changes -
          Attachment hdfs727.patch [ 12422956 ]
          Hide
          dhruba borthakur added a comment -

          does this occur on a [articular OS platform? solaris? redhat linux?

          Show
          dhruba borthakur added a comment - does this occur on a [articular OS platform? solaris? redhat linux?
          Hide
          Eli Collins added a comment -

          I saw this bug on 64-bit linux, but you could see it on any 64-bit system.

          invokeMethod(env, &jVal, &jExc, INSTANCE, jConfiguration, 
                                        HADOOP_CONF, "getLong", "(Ljava/lang/String;J)J",
                                       jStrBlockSize, 67108864))  {
          

          In the above code the value 67108864 will be passed as an int since the literal fits in an int (the compiler won't do type promotion here because it doesn't know argument types for variadic functions). Meanwhile invokeMethod passes the va_list to CallObjectMethodV which expects a jlong (8 bytes). The behavior is undefined, in practice it read 8 bytes when accessing this parameter, which ends up being the 4 byte value plus 4 bytes of whatever was passed next to it.

          Show
          Eli Collins added a comment - I saw this bug on 64-bit linux, but you could see it on any 64-bit system. invokeMethod(env, &jVal, &jExc, INSTANCE, jConfiguration, HADOOP_CONF, "getLong" , "(Ljava/lang/ String ;J)J" , jStrBlockSize, 67108864)) { In the above code the value 67108864 will be passed as an int since the literal fits in an int (the compiler won't do type promotion here because it doesn't know argument types for variadic functions). Meanwhile invokeMethod passes the va_list to CallObjectMethodV which expects a jlong (8 bytes). The behavior is undefined, in practice it read 8 bytes when accessing this parameter, which ends up being the 4 byte value plus 4 bytes of whatever was passed next to it.
          Hide
          dhruba borthakur added a comment -

          This seems to be a serious problem. I would like to backport it to 0.20.2 and 0.21. Can we enhannce hdfs_test.c to include a unit test to test this patch?

          Show
          dhruba borthakur added a comment - This seems to be a serious problem. I would like to backport it to 0.20.2 and 0.21. Can we enhannce hdfs_test.c to include a unit test to test this patch?
          dhruba borthakur made changes -
          Fix Version/s 0.20.2 [ 12314204 ]
          Fix Version/s 0.21.0 [ 12314046 ]
          Hide
          Eli Collins added a comment -

          hdfs_test doesn't catch this, I confirmed hdfsOpenFile as used in the test does set the block size incorrectly but I suspect the create path isn't going through the BlockManager (which would fail the write because no DN matches the requested block size). I'll update the test to catch this bug.

          Show
          Eli Collins added a comment - hdfs_test doesn't catch this, I confirmed hdfsOpenFile as used in the test does set the block size incorrectly but I suspect the create path isn't going through the BlockManager (which would fail the write because no DN matches the requested block size). I'll update the test to catch this bug.
          Hide
          dhruba borthakur added a comment -

          Hi Eli, Thanks for offering to update the test case, will wait for that one. This is something that is worth putting into the 0.20.3 release.

          Show
          dhruba borthakur added a comment - Hi Eli, Thanks for offering to update the test case, will wait for that one. This is something that is worth putting into the 0.20.3 release.
          Hide
          Eli Collins added a comment -

          Hey Dhruba,

          Now that I can run the libhdfs test on trunk (HDFS-756), I ran the libhdfs test w/o the patch in this jira and confirmed that on an ubuntu 9.10 64-bit host the test fails due to this bug. Adding
          fprintf(stderr, "jBlockSize=%lld\n", jBlockSize); in hdfsOpenFile shows the corrupt value in test output, and the failure (could only be replicated to 0 nodes) is the same failure I saw before (there's no node that will accept this large block size).

          [exec] jBlockSize 47403621154816
          ...
          [exec] 09/11/16 20:08:06 WARN hdfs.DFSClient: DataStreamer Exception: org.apache.hadoop.ipc.RemoteException: java.io.IOException:
          File /tmp/testfile.txt could only be replicated to 0 nodes, instead of 1

          The patch still applies against trunk and 20.1 and 20.2.

          Thanks,
          Eli

          Show
          Eli Collins added a comment - Hey Dhruba, Now that I can run the libhdfs test on trunk ( HDFS-756 ), I ran the libhdfs test w/o the patch in this jira and confirmed that on an ubuntu 9.10 64-bit host the test fails due to this bug. Adding fprintf(stderr, "jBlockSize=%lld\n", jBlockSize); in hdfsOpenFile shows the corrupt value in test output, and the failure (could only be replicated to 0 nodes) is the same failure I saw before (there's no node that will accept this large block size). [exec] jBlockSize 47403621154816 ... [exec] 09/11/16 20:08:06 WARN hdfs.DFSClient: DataStreamer Exception: org.apache.hadoop.ipc.RemoteException: java.io.IOException: File /tmp/testfile.txt could only be replicated to 0 nodes, instead of 1 The patch still applies against trunk and 20.1 and 20.2. Thanks, Eli
          Eli Collins made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12422956/hdfs727.patch
          against trunk revision 881017.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          -1 patch. The patch command could not apply the patch.

          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/77/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12422956/hdfs727.patch against trunk revision 881017. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/77/console This message is automatically generated.
          Hide
          Eli Collins added a comment -

          Looks like the diff to hdfsJniHelper.h was already applied, here's the patch again w/o that diff.

          Show
          Eli Collins added a comment - Looks like the diff to hdfsJniHelper.h was already applied, here's the patch again w/o that diff.
          Eli Collins made changes -
          Attachment hdfs727.patch [ 12425186 ]
          Eli Collins made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Eli Collins made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12425186/hdfs727.patch
          against trunk revision 881695.

          +1 @author. The patch does not contain any @author tags.

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          -1 release audit. The applied patch generated 126 release audit warnings (more than the trunk's current 0 warnings).

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/78/testReport/
          Release audit warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/78/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/78/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/78/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/78/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12425186/hdfs727.patch against trunk revision 881695. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. -1 release audit. The applied patch generated 126 release audit warnings (more than the trunk's current 0 warnings). -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/78/testReport/ Release audit warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/78/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/78/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/78/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/78/console This message is automatically generated.
          Eli Collins made changes -
          Link This issue blocks HDFS-756 [ HDFS-756 ]
          Hide
          Konstantin Boudnik added a comment -

          +1 patch looks good.

          I've verified if it works for HDFS-756 and it seems to be doing the job. I'm going to wait till tomorrow if anyone wants to comment on this and will commit it.

          Show
          Konstantin Boudnik added a comment - +1 patch looks good. I've verified if it works for HDFS-756 and it seems to be doing the job. I'm going to wait till tomorrow if anyone wants to comment on this and will commit it.
          Hide
          Konstantin Boudnik added a comment -

          Clearly, the problem with the test is unrelated. However, I have executed the failing test case from test-patch run with this patch in place and test test is passing ok.

          Show
          Konstantin Boudnik added a comment - Clearly, the problem with the test is unrelated. However, I have executed the failing test case from test-patch run with this patch in place and test test is passing ok.
          Hide
          Konstantin Boudnik added a comment -

          This issue doesn't apply to 0.21 for there are needed sources available.

          Show
          Konstantin Boudnik added a comment - This issue doesn't apply to 0.21 for there are needed sources available.
          Konstantin Boudnik made changes -
          Fix Version/s 0.22.0 [ 12314241 ]
          Fix Version/s 0.21.0 [ 12314046 ]
          Fix Version/s 0.20.2 [ 12314204 ]
          Affects Version/s 0.22.0 [ 12314241 ]
          Component/s contrib/libhdfs [ 12313126 ]
          Hide
          Konstantin Boudnik added a comment -

          I've just committed this. Thanks Eli.

          Show
          Konstantin Boudnik added a comment - I've just committed this. Thanks Eli.
          Konstantin Boudnik made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Hadoop Flags [Reviewed]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #117 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/117/)
          . bug setting block size hdfsOpenFile. Contributed by Eli Collins.

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #117 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/117/ ) . bug setting block size hdfsOpenFile. Contributed by Eli Collins.
          Hide
          Hudson added a comment -

          Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #121 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/121/)
          . bug setting block size hdfsOpenFile. Contributed by Eli Collins.

          Show
          Hudson added a comment - Integrated in Hdfs-Patch-h5.grid.sp2.yahoo.net #121 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h5.grid.sp2.yahoo.net/121/ ) . bug setting block size hdfsOpenFile. Contributed by Eli Collins.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #150 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/150/)

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #150 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/150/ )
          Hide
          Eli Collins added a comment -

          It doesn't look like the change made it into branch-0.20. Cos, can you check it in there?

          Show
          Eli Collins added a comment - It doesn't look like the change made it into branch-0.20. Cos, can you check it in there?
          Eli Collins made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Hide
          Hudson added a comment -

          Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #81 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/81/)

          Show
          Hudson added a comment - Integrated in Hdfs-Patch-h2.grid.sp2.yahoo.net #81 (See http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-h2.grid.sp2.yahoo.net/81/ )
          Eli Collins made changes -
          Fix Version/s 0.20.2 [ 12314204 ]
          Fix Version/s 0.22.0 [ 12314241 ]
          Affects Version/s 0.20.2 [ 12314204 ]
          Affects Version/s 0.22.0 [ 12314241 ]
          Priority Major [ 3 ] Blocker [ 1 ]
          Chris Douglas made changes -
          Fix Version/s 0.20.2 [ 12314204 ]
          Hide
          Eli Collins added a comment -

          I merged HDFS-727 from trunk into branch-0.20, ran the libhdfs test and committed.

          Show
          Eli Collins added a comment - I merged HDFS-727 from trunk into branch-0.20, ran the libhdfs test and committed.
          Eli Collins made changes -
          Status Reopened [ 4 ] Resolved [ 5 ]
          Fix Version/s 0.20.3 [ 12314814 ]
          Fix Version/s 0.22.0 [ 12314241 ]
          Resolution Fixed [ 1 ]
          Konstantin Shvachko made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Patch Available Patch Available Open Open
          2d 13h 22m 1 Eli Collins 19/Nov/09 17:44
          Open Open Patch Available Patch Available
          25d 6h 46m 2 Eli Collins 19/Nov/09 17:45
          Patch Available Patch Available Resolved Resolved
          1d 3h 43m 1 Konstantin Boudnik 20/Nov/09 21:28
          Resolved Resolved Reopened Reopened
          12d 2h 43m 1 Eli Collins 03/Dec/09 00:11
          Reopened Reopened Resolved Resolved
          345d 5h 47m 1 Eli Collins 13/Nov/10 05:59
          Resolved Resolved Closed Closed
          394d 20m 1 Konstantin Shvachko 12/Dec/11 06:19

            People

            • Assignee:
              Eli Collins
              Reporter:
              Eli Collins
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development