Details

    • Hadoop Flags:
      Reviewed

      Description

      When writing a file, the pipeline status read timeouts for datanodes are not set up properly.

      1. hdfs-561-0.20s.patch
        2 kB
        Suresh Srinivas
      2. hdfs-561-0.20-append.txt
        3 kB
        Todd Lipcon
      3. h561-02.patch
        20 kB
        Kan Zhang
      4. h561-01.patch
        20 kB
        Kan Zhang

        Issue Links

          Activity

          Kan Zhang created issue -
          Kan Zhang made changes -
          Field Original Value New Value
          Attachment h561-01.patch [ 12417292 ]
          Hide
          Kan Zhang added a comment -

          Attaching a patch, which
          1. Fix datanode READ TIMEOUT setup to match that of the DFSClient.
          2. Incorporate tests 01-16 of Nicholas's test patch for HDFS-483 and make them pass.

          Show
          Kan Zhang added a comment - Attaching a patch, which 1. Fix datanode READ TIMEOUT setup to match that of the DFSClient. 2. Incorporate tests 01-16 of Nicholas's test patch for HDFS-483 and make them pass.
          Kan Zhang 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/12417292/h561-01.patch
          against trunk revision 806746.

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

          +1 tests included. The patch appears to include 9 new or modified tests.

          +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 does not increase the total number of release audit 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-vesta.apache.org/82/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/82/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/82/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/82/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/12417292/h561-01.patch against trunk revision 806746. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 9 new or modified tests. +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 does not increase the total number of release audit 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-vesta.apache.org/82/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/82/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/82/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/82/console This message is automatically generated.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          patch look good except that there are un-used imports in DFSClientAspects.

          Some suggestions:
          Currently, both ActionContainer.run(..) and Action.run(..) throw IOException. However, some implementation like VerificationAction may not throw IOException. My suggestion is to add a generic type to Action and ActionContainer as shown below.

            public static interface Action<T, E extends Exception> {
              /** Run the action with the parameter. */
              public void run(T parameter) throws E;
            }
          
            /** An ActionContainer contains at most one action. */
            public static class ActionContainer<T, E extends Exception> {
              private Action<T, E> action;
          
              /** Create an empty container. */
              public ActionContainer() {}
          
              /** Set action. */
              public void set(Action<T, E> a) {action = a;}
          
              /** Run the action if it exists. */
              public void run(T obj) throws E {
                if (action != null) {
                  action.run(obj);
                }
              }
            }
          

          We may do this in a separated issue.

          Show
          Tsz Wo Nicholas Sze added a comment - patch look good except that there are un-used imports in DFSClientAspects. Some suggestions: Currently, both ActionContainer.run(..) and Action.run(..) throw IOException. However, some implementation like VerificationAction may not throw IOException. My suggestion is to add a generic type to Action and ActionContainer as shown below. public static interface Action<T, E extends Exception> { /** Run the action with the parameter. */ public void run(T parameter) throws E; } /** An ActionContainer contains at most one action. */ public static class ActionContainer<T, E extends Exception> { private Action<T, E> action; /** Create an empty container. */ public ActionContainer() {} /** Set action. */ public void set(Action<T, E> a) {action = a;} /** Run the action if it exists. */ public void run(T obj) throws E { if (action != null ) { action.run(obj); } } } We may do this in a separated issue.
          Hide
          Kan Zhang added a comment -

          attaching a new patch, which removes the unnecessary imports. Otherwise, it's identical to the first patch. Thanks for your suggestion. Will address it when I add more test cases.

          Show
          Kan Zhang added a comment - attaching a new patch, which removes the unnecessary imports. Otherwise, it's identical to the first patch. Thanks for your suggestion. Will address it when I add more test cases.
          Kan Zhang made changes -
          Attachment h561-02.patch [ 12417538 ]
          Hide
          Tsz Wo Nicholas Sze added a comment -

          +1 the new patch looks good.

          The failed test, TestServiceLevelAuthorization, is not related.

          Show
          Tsz Wo Nicholas Sze added a comment - +1 the new patch looks good. The failed test, TestServiceLevelAuthorization, is not related.
          Tsz Wo Nicholas Sze made changes -
          Hadoop Flags [Reviewed]
          Fix Version/s 0.21.0 [ 12314046 ]
          Component/s test [ 12312916 ]
          Hide
          Tsz Wo Nicholas Sze added a comment -

          I have committed this. Thanks, Kan!

          Show
          Tsz Wo Nicholas Sze added a comment - I have committed this. Thanks, Kan!
          Tsz Wo Nicholas Sze made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Hudson added a comment -

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

          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #65 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Hdfs-trunk/65/ )
          Todd Lipcon made changes -
          Link This issue relates to HDFS-770 [ HDFS-770 ]
          Hide
          Todd Lipcon added a comment -

          I think this fix should be applied to branch-20 as well. It's a fairly minor change, and although we wouldn't be able to use the new FI tests, I think it's a pretty straightforward error in the existing logic (I independently found this bug in branch 20 last night, fixed in same way, then found this JIRA).

          Show
          Todd Lipcon added a comment - I think this fix should be applied to branch-20 as well. It's a fairly minor change, and although we wouldn't be able to use the new FI tests, I think it's a pretty straightforward error in the existing logic (I independently found this bug in branch 20 last night, fixed in same way, then found this JIRA).
          Todd Lipcon made changes -
          Affects Version/s 0.20-append [ 12315103 ]
          Hide
          Todd Lipcon added a comment -

          Patch for 0.20-append branch

          Show
          Todd Lipcon added a comment - Patch for 0.20-append branch
          Todd Lipcon made changes -
          Attachment hdfs-561-0.20-append.txt [ 12446924 ]
          dhruba borthakur made changes -
          Fix Version/s 0.20-append [ 12315103 ]
          Tom White made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Suresh Srinivas made changes -
          Fix Version/s 0.20.205.0 [ 12316392 ]
          Hide
          Suresh Srinivas added a comment -

          Attaching a patch 0.20-security for 0.20.205 release.

          Show
          Suresh Srinivas added a comment - Attaching a patch 0.20-security for 0.20.205 release.
          Suresh Srinivas made changes -
          Attachment hdfs-561-0.20s.patch [ 12491259 ]
          Hide
          Jitendra Nath Pandey added a comment -

          +1 for 0.20-security patch.

          Show
          Jitendra Nath Pandey added a comment - +1 for 0.20-security patch.

            People

            • Assignee:
              Kan Zhang
              Reporter:
              Kan Zhang
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development