Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-12909

Change ipc.Client to support asynchronous calls

    Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: ipc
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      In ipc.Client, the underlying mechanism is already supporting asynchronous calls – the calls shares a connection, the call requests are sent using a thread pool and the responses can be out of order. Indeed, synchronous call is implemented by invoking wait() in the caller thread in order to wait for the server response.

      In this JIRA, we change ipc.Client to support asynchronous mode. In asynchronous mode, it return once the request has been sent out but not wait for the response from the server.

      1. HADOOP-12909-HDFS-9924.000.patch
        31 kB
        Xiaobing Zhou
      2. HADOOP-12909-HDFS-9924.001.patch
        31 kB
        Xiaobing Zhou
      3. HADOOP-12909-HDFS-9924.002.patch
        29 kB
        Xiaobing Zhou
      4. HADOOP-12909-HDFS-9924.003.patch
        63 kB
        Xiaobing Zhou
      5. HADOOP-12909-HDFS-9924.004.patch
        32 kB
        Xiaobing Zhou
      6. HADOOP-12909-HDFS-9924.005.patch
        32 kB
        Xiaobing Zhou
      7. HADOOP-12909-HDFS-9924.006.patch
        33 kB
        Xiaobing Zhou
      8. HADOOP-12909-HDFS-9924.007.patch
        33 kB
        Xiaobing Zhou
      9. HADOOP-12909-HDFS-9924.008.patch
        32 kB
        Xiaobing Zhou
      10. HADOOP-12909-HDFS-9924.009.patch
        20 kB
        Xiaobing Zhou

        Issue Links

          Activity

          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          This sounds related to some of the work Siddharth Seth was doing at HADOOP-11552.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - This sounds related to some of the work Siddharth Seth was doing at HADOOP-11552 .
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Thanks, Vinod Kumar Vavilapalli and Siddharth Seth. HADOOP-11552 looks like a useful server side improvement.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Thanks, Vinod Kumar Vavilapalli and Siddharth Seth . HADOOP-11552 looks like a useful server side improvement.
          Hide
          sseth Siddharth Seth added a comment -

          There are potential problems with supporting client side calls without fixing the server side - the main one being that all handler threads on the server can end up getting blocked. Of course, the same would happen if the client app were to create it's own threads and make remote calls (FileSystem for instance).
          The future based approach mentioned here and other related jiras ends up simplifying client code; however frameworks need to be aware of the potential affect on the server.

          Show
          sseth Siddharth Seth added a comment - There are potential problems with supporting client side calls without fixing the server side - the main one being that all handler threads on the server can end up getting blocked. Of course, the same would happen if the client app were to create it's own threads and make remote calls (FileSystem for instance). The future based approach mentioned here and other related jiras ends up simplifying client code; however frameworks need to be aware of the potential affect on the server.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Siddharth Seth, could you update your patch in HADOOP-11552? I will review it. Thanks.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Siddharth Seth , could you update your patch in HADOOP-11552 ? I will review it. Thanks.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          I posted the initial patch V000 for review, thanks.

          Show
          xiaobingo Xiaobing Zhou added a comment - I posted the initial patch V000 for review, thanks.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 9s HADOOP-12909 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12793432/HADOOP-12909-HDFS-9924.000.patch
          JIRA Issue HADOOP-12909
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8848/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 9s HADOOP-12909 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12793432/HADOOP-12909-HDFS-9924.000.patch JIRA Issue HADOOP-12909 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8848/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Xiaobing Zhou, thanks for posting a patch. Some comments:

          • We should not add the new conf fs.client.in.asynchronous.mode. Let's simply add a setAsynchronous somewhere. It may be implemented by a ThreadLocal variable (similar to returnFuture).
          • Let's rename isInAsynchronousMode to isAsynchronous and returnFuture to returnValue
          • We don't need recvReponseExecutor for receiving responses. We could implement a Future subclass so that get() will invoke getRpcResponse(call, connection) and then return the return value of the call.
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Xiaobing Zhou , thanks for posting a patch. Some comments: We should not add the new conf fs.client.in.asynchronous.mode. Let's simply add a setAsynchronous somewhere. It may be implemented by a ThreadLocal variable (similar to returnFuture). Let's rename isInAsynchronousMode to isAsynchronous and returnFuture to returnValue We don't need recvReponseExecutor for receiving responses. We could implement a Future subclass so that get() will invoke getRpcResponse(call, connection) and then return the return value of the call.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          V001 only rebased trunk since V000 failed to apply on it.

          Show
          xiaobingo Xiaobing Zhou added a comment - V001 only rebased trunk since V000 failed to apply on it.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 10m 54s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 25s Maven dependency ordering for branch
          +1 mvninstall 6m 41s trunk passed
          +1 compile 5m 37s trunk passed with JDK v1.8.0_74
          +1 compile 6m 31s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 5s trunk passed
          +1 mvnsite 1m 47s trunk passed
          +1 mvneclipse 0m 26s trunk passed
          +1 findbugs 3m 30s trunk passed
          +1 javadoc 1m 58s trunk passed with JDK v1.8.0_74
          +1 javadoc 2m 48s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 1m 26s the patch passed
          +1 compile 5m 37s the patch passed with JDK v1.8.0_74
          -1 javac 7m 17s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738)
          +1 javac 5m 37s the patch passed
          +1 compile 6m 33s the patch passed with JDK v1.7.0_95
          -1 javac 13m 50s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 1 new + 733 unchanged - 1 fixed = 734 total (was 734)
          +1 javac 6m 33s the patch passed
          -1 checkstyle 1m 5s root: patch generated 10 new + 161 unchanged - 2 fixed = 171 total (was 163)
          +1 mvnsite 1m 45s the patch passed
          +1 mvneclipse 0m 27s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 xml 0m 0s The patch has no ill-formed XML file.
          +1 findbugs 3m 56s the patch passed
          +1 javadoc 1m 53s the patch passed with JDK v1.8.0_74
          +1 javadoc 2m 47s the patch passed with JDK v1.7.0_95
          +1 unit 7m 2s hadoop-common in the patch passed with JDK v1.8.0_74.
          -1 unit 54m 23s hadoop-hdfs in the patch failed with JDK v1.8.0_74.
          +1 unit 7m 34s hadoop-common in the patch passed with JDK v1.7.0_95.
          -1 unit 52m 14s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 26s Patch does not generate ASF License warnings.
          190m 27s



          Reason Tests
          JDK v1.8.0_74 Failed junit tests hadoop.tools.TestHdfsConfigFields
            hadoop.hdfs.TestCrcCorruption
            hadoop.hdfs.TestHFlush
          JDK v1.7.0_95 Failed junit tests hadoop.tools.TestHdfsConfigFields



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12793627/HADOOP-12909-HDFS-9924.001.patch
          JIRA Issue HADOOP-12909
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 8779fe91f1a6 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6529c875
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt
          javac root-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 10m 54s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 25s Maven dependency ordering for branch +1 mvninstall 6m 41s trunk passed +1 compile 5m 37s trunk passed with JDK v1.8.0_74 +1 compile 6m 31s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 5s trunk passed +1 mvnsite 1m 47s trunk passed +1 mvneclipse 0m 26s trunk passed +1 findbugs 3m 30s trunk passed +1 javadoc 1m 58s trunk passed with JDK v1.8.0_74 +1 javadoc 2m 48s trunk passed with JDK v1.7.0_95 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 26s the patch passed +1 compile 5m 37s the patch passed with JDK v1.8.0_74 -1 javac 7m 17s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738) +1 javac 5m 37s the patch passed +1 compile 6m 33s the patch passed with JDK v1.7.0_95 -1 javac 13m 50s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 1 new + 733 unchanged - 1 fixed = 734 total (was 734) +1 javac 6m 33s the patch passed -1 checkstyle 1m 5s root: patch generated 10 new + 161 unchanged - 2 fixed = 171 total (was 163) +1 mvnsite 1m 45s the patch passed +1 mvneclipse 0m 27s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 xml 0m 0s The patch has no ill-formed XML file. +1 findbugs 3m 56s the patch passed +1 javadoc 1m 53s the patch passed with JDK v1.8.0_74 +1 javadoc 2m 47s the patch passed with JDK v1.7.0_95 +1 unit 7m 2s hadoop-common in the patch passed with JDK v1.8.0_74. -1 unit 54m 23s hadoop-hdfs in the patch failed with JDK v1.8.0_74. +1 unit 7m 34s hadoop-common in the patch passed with JDK v1.7.0_95. -1 unit 52m 14s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 26s Patch does not generate ASF License warnings. 190m 27s Reason Tests JDK v1.8.0_74 Failed junit tests hadoop.tools.TestHdfsConfigFields   hadoop.hdfs.TestCrcCorruption   hadoop.hdfs.TestHFlush JDK v1.7.0_95 Failed junit tests hadoop.tools.TestHdfsConfigFields Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12793627/HADOOP-12909-HDFS-9924.001.patch JIRA Issue HADOOP-12909 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 8779fe91f1a6 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6529c875 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt javac root-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8856/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          Tsz Wo Nicholas Sze thanks for review. The patch V002 covered your comments.

          Show
          xiaobingo Xiaobing Zhou added a comment - Tsz Wo Nicholas Sze thanks for review. The patch V002 covered your comments.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 9s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 6m 56s trunk passed
          +1 compile 6m 15s trunk passed with JDK v1.8.0_74
          +1 compile 7m 1s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 22s trunk passed
          +1 mvnsite 0m 59s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 1m 38s trunk passed
          +1 javadoc 0m 56s trunk passed with JDK v1.8.0_74
          +1 javadoc 1m 4s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 42s the patch passed
          +1 compile 6m 17s the patch passed with JDK v1.8.0_74
          -1 javac 10m 49s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738)
          +1 javac 6m 17s the patch passed
          +1 compile 6m 48s the patch passed with JDK v1.7.0_95
          -1 javac 17m 37s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 1 new + 733 unchanged - 1 fixed = 734 total (was 734)
          +1 javac 6m 48s the patch passed
          -1 checkstyle 0m 21s hadoop-common-project/hadoop-common: patch generated 6 new + 84 unchanged - 2 fixed = 90 total (was 86)
          +1 mvnsite 0m 58s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 1m 50s the patch passed
          +1 javadoc 0m 55s the patch passed with JDK v1.8.0_74
          +1 javadoc 1m 10s the patch passed with JDK v1.7.0_95
          +1 unit 7m 46s hadoop-common in the patch passed with JDK v1.8.0_74.
          -1 unit 7m 45s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 25s Patch does not generate ASF License warnings.
          61m 53s



          Reason Tests
          JDK v1.7.0_95 Failed junit tests hadoop.net.TestClusterTopology



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12793920/HADOOP-12909-HDFS-9924.002.patch
          JIRA Issue HADOOP-12909
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux fc72240e370e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 02a250d
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt
          javac root-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 9s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 6m 56s trunk passed +1 compile 6m 15s trunk passed with JDK v1.8.0_74 +1 compile 7m 1s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 59s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 1m 38s trunk passed +1 javadoc 0m 56s trunk passed with JDK v1.8.0_74 +1 javadoc 1m 4s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 42s the patch passed +1 compile 6m 17s the patch passed with JDK v1.8.0_74 -1 javac 10m 49s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738) +1 javac 6m 17s the patch passed +1 compile 6m 48s the patch passed with JDK v1.7.0_95 -1 javac 17m 37s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 1 new + 733 unchanged - 1 fixed = 734 total (was 734) +1 javac 6m 48s the patch passed -1 checkstyle 0m 21s hadoop-common-project/hadoop-common: patch generated 6 new + 84 unchanged - 2 fixed = 90 total (was 86) +1 mvnsite 0m 58s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 1m 50s the patch passed +1 javadoc 0m 55s the patch passed with JDK v1.8.0_74 +1 javadoc 1m 10s the patch passed with JDK v1.7.0_95 +1 unit 7m 46s hadoop-common in the patch passed with JDK v1.8.0_74. -1 unit 7m 45s hadoop-common in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 25s Patch does not generate ASF License warnings. 61m 53s Reason Tests JDK v1.7.0_95 Failed junit tests hadoop.net.TestClusterTopology Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12793920/HADOOP-12909-HDFS-9924.002.patch JIRA Issue HADOOP-12909 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux fc72240e370e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 02a250d Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt javac root-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8864/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          The new patch looks great! I like the idea of using AbstractFuture since it is also a ListenableFuture. Some comments:

          • As described in the AbstractFuture API, we should use set(..) and setException(..) so that other methods such as isDone() will also work.
          • TestIPC is changed so that TestAsyncIPC can reuse the code. It is better not to change TestIPC since we want to make use the existing behaviors still work. Let's just copy the code to TestAsyncIPC.
          • There are some format changes. Please revert them.
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - The new patch looks great! I like the idea of using AbstractFuture since it is also a ListenableFuture. Some comments: As described in the AbstractFuture API , we should use set(..) and setException(..) so that other methods such as isDone() will also work. TestIPC is changed so that TestAsyncIPC can reuse the code. It is better not to change TestIPC since we want to make use the existing behaviors still work. Let's just copy the code to TestAsyncIPC. There are some format changes. Please revert them.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          V003 fixed issues you mentioned, thanks Tsz Wo Nicholas Sze.

          Show
          xiaobingo Xiaobing Zhou added a comment - V003 fixed issues you mentioned, thanks Tsz Wo Nicholas Sze .
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 39s trunk passed
          +1 compile 6m 55s trunk passed with JDK v1.8.0_74
          +1 compile 7m 47s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 27s trunk passed
          +1 mvnsite 1m 13s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 1m 47s trunk passed
          +1 javadoc 1m 3s trunk passed with JDK v1.8.0_74
          +1 javadoc 1m 13s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 49s the patch passed
          +1 compile 6m 50s the patch passed with JDK v1.8.0_74
          -1 javac 11m 56s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738)
          +1 javac 6m 50s the patch passed
          +1 compile 6m 36s the patch passed with JDK v1.7.0_95
          +1 javac 6m 36s the patch passed
          -1 checkstyle 0m 21s hadoop-common-project/hadoop-common: patch generated 5 new + 85 unchanged - 1 fixed = 90 total (was 86)
          +1 mvnsite 0m 55s the patch passed
          +1 mvneclipse 0m 14s the patch passed
          -1 whitespace 0m 0s The patch has 82 line(s) that end in whitespace. Use git apply --whitespace=fix.
          -1 whitespace 0m 2s The patch has 1 line(s) with tabs.
          +1 findbugs 1m 51s the patch passed
          +1 javadoc 0m 51s the patch passed with JDK v1.8.0_74
          +1 javadoc 1m 3s the patch passed with JDK v1.7.0_95
          -1 unit 6m 54s hadoop-common in the patch failed with JDK v1.8.0_74.
          -1 unit 7m 11s hadoop-common in the patch failed with JDK v1.7.0_95.
          -1 asflicense 0m 24s Patch generated 2 ASF License warnings.
          63m 56s



          Reason Tests
          JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker
          JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12794324/HADOOP-12909-HDFS-9924.003.patch
          JIRA Issue HADOOP-12909
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c1eb6d7cafe4 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 33239c9
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/whitespace-eol.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/whitespace-tabs.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 39s trunk passed +1 compile 6m 55s trunk passed with JDK v1.8.0_74 +1 compile 7m 47s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 27s trunk passed +1 mvnsite 1m 13s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 1m 47s trunk passed +1 javadoc 1m 3s trunk passed with JDK v1.8.0_74 +1 javadoc 1m 13s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 49s the patch passed +1 compile 6m 50s the patch passed with JDK v1.8.0_74 -1 javac 11m 56s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738) +1 javac 6m 50s the patch passed +1 compile 6m 36s the patch passed with JDK v1.7.0_95 +1 javac 6m 36s the patch passed -1 checkstyle 0m 21s hadoop-common-project/hadoop-common: patch generated 5 new + 85 unchanged - 1 fixed = 90 total (was 86) +1 mvnsite 0m 55s the patch passed +1 mvneclipse 0m 14s the patch passed -1 whitespace 0m 0s The patch has 82 line(s) that end in whitespace. Use git apply --whitespace=fix. -1 whitespace 0m 2s The patch has 1 line(s) with tabs. +1 findbugs 1m 51s the patch passed +1 javadoc 0m 51s the patch passed with JDK v1.8.0_74 +1 javadoc 1m 3s the patch passed with JDK v1.7.0_95 -1 unit 6m 54s hadoop-common in the patch failed with JDK v1.8.0_74. -1 unit 7m 11s hadoop-common in the patch failed with JDK v1.7.0_95. -1 asflicense 0m 24s Patch generated 2 ASF License warnings. 63m 56s Reason Tests JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12794324/HADOOP-12909-HDFS-9924.003.patch JIRA Issue HADOOP-12909 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c1eb6d7cafe4 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 33239c9 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/whitespace-eol.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/whitespace-tabs.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/testReport/ asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8878/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          At first I thought this wasn't thread-safe, but it's really using thread-local state to do async callls. Even so, it's not, well, pleasant. Is there any cleaner way to do it?

          Code-wise, if you are playing with Client, have it implement AutoCloseable with close() a final method invoking stop(). You'll benefit that immediately in the tests, as you can use try-with-resources to automatically get the client served up.

          Show
          stevel@apache.org Steve Loughran added a comment - At first I thought this wasn't thread-safe, but it's really using thread-local state to do async callls. Even so, it's not, well, pleasant. Is there any cleaner way to do it? Code-wise, if you are playing with Client , have it implement AutoCloseable with close() a final method invoking stop() . You'll benefit that immediately in the tests, as you can use try-with-resources to automatically get the client served up.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          > ... Is there any cleaner way to do it?

          The reason of being implemented using ThreadLocal is that there are no easy ways to get the ipc.Client object from the caller, say DFSClient. The ipc.Client object is hidden very deep by Java Proxy and the RpcEngine layer. If you could find some better ways to do it, please kindly let us know.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - > ... Is there any cleaner way to do it? The reason of being implemented using ThreadLocal is that there are no easy ways to get the ipc.Client object from the caller, say DFSClient. The ipc.Client object is hidden very deep by Java Proxy and the RpcEngine layer. If you could find some better ways to do it, please kindly let us know.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          > V003 fixed issues you mentioned, ...

          Xiaobing Zhou, thanks for the update. Some comments on the new patch.

          • The patch still has quite a few format changes such as shown below.
            @@ -176,7 +190,7 @@ synchronized ExecutorService refAndGetInstance() {
                   
                   return clientExecutor;
                 }
            -    
            +
                 /**
                  * Cleanup Executor on which IPC calls' parameters are sent.
                  * If reference counter is zero, this method discards the
            
          • For the new test, we should reuse the code in TestIPC if it does not require logic changes in it. Sorry that I was not clear in my last comment. For example, we should reuse NetworkTraces and other static methods by simply removing the "private" declarations in TestIPC.
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - > V003 fixed issues you mentioned, ... Xiaobing Zhou , thanks for the update. Some comments on the new patch. The patch still has quite a few format changes such as shown below. @@ -176,7 +190,7 @@ synchronized ExecutorService refAndGetInstance() { return clientExecutor; } - + /** * Cleanup Executor on which IPC calls' parameters are sent. * If reference counter is zero, this method discards the For the new test, we should reuse the code in TestIPC if it does not require logic changes in it. Sorry that I was not clear in my last comment. For example, we should reuse NetworkTraces and other static methods by simply removing the "private" declarations in TestIPC.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          One more comment: the get() method should always return super.get() as below. In case of exception, super.get() will throw it for us.

          +        public Writable get() throws InterruptedException, ExecutionException {
          +          try {
          +            set(getRpcResponse(call, connection));
          +          } catch (IOException ie) {
          +            setException(ie);
          +          }
          +          return super.get();
          +        }
          
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - One more comment: the get() method should always return super.get() as below. In case of exception, super.get() will throw it for us. + public Writable get() throws InterruptedException, ExecutionException { + try { + set(getRpcResponse(call, connection)); + } catch (IOException ie) { + setException(ie); + } + return super .get(); + }
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I don't know of any easy way, though this could be an opportunity to expose this functionality. In particular, could this be set in the constructor? I know it's not easy with some calls potentially being sync and some being async, but I'd propose having separate instances here. Existing sync FS, through FileSystem.getFileSystem; the async one through FileSystem.getAsyncFilesystem() where the latter is (a) refcounted and (b) fully asyc

          I do think it's ugly, and worry that it's dangerously easy to share a client across two threads, with consequences.

          I think you may want to involve other people with deep involvement in the IPC layer (Sanjay Radia for their insight.

          Show
          stevel@apache.org Steve Loughran added a comment - I don't know of any easy way, though this could be an opportunity to expose this functionality. In particular, could this be set in the constructor? I know it's not easy with some calls potentially being sync and some being async, but I'd propose having separate instances here. Existing sync FS, through FileSystem.getFileSystem ; the async one through FileSystem.getAsyncFilesystem() where the latter is (a) refcounted and (b) fully asyc I do think it's ugly, and worry that it's dangerously easy to share a client across two threads, with consequences. I think you may want to involve other people with deep involvement in the IPC layer ( Sanjay Radia for their insight.
          Hide
          sanjay.radia Sanjay Radia added a comment -

          I haven't had a chance to look at the patch or review all the comments, but wanted to bring attention to one issue wrt async rpc that is well known by implementors and practitioners of message passing & rpc systems (excuse me if this has already been covered):

          • One needs to watch out for buffer management. ie. aync rpc/message passing has the potential to use up memory for buffering the messages. This is prevented in Sync rpc systems:
            • the sender (client) blocks and cannot flood the receiver unless it uses threads
            • the receiver (server) is guaranteed that the sender (ie client) is waiting to receive and if it has died then the reply can be discarded.

          With asyn rpc , my suggestion is to consider something along the following lines:

          • the client needs to allocate some buffer (or space for it) where replies are stored. On each async rpc call, it passes a ref to this buffer for storing replies. If the client does not pick up the replies fast enough then his next async call using that buffer space will block.
          • Note this makes the clients code tricky in what to do if it is blocked since one must ensure that a deadlock or starvation does not happen (but async messaging has always been tricky which is why cs community went with sync rpc). Note this problem does not arise on server side async-rpc since the client is blocked waiting for reply (unless the client also did async call but in that case its buffer, as per my suggestion, must be there to store the reply).
          Show
          sanjay.radia Sanjay Radia added a comment - I haven't had a chance to look at the patch or review all the comments, but wanted to bring attention to one issue wrt async rpc that is well known by implementors and practitioners of message passing & rpc systems (excuse me if this has already been covered): One needs to watch out for buffer management. ie. aync rpc/message passing has the potential to use up memory for buffering the messages. This is prevented in Sync rpc systems: the sender (client) blocks and cannot flood the receiver unless it uses threads the receiver (server) is guaranteed that the sender (ie client) is waiting to receive and if it has died then the reply can be discarded. With asyn rpc , my suggestion is to consider something along the following lines: the client needs to allocate some buffer (or space for it) where replies are stored. On each async rpc call, it passes a ref to this buffer for storing replies. If the client does not pick up the replies fast enough then his next async call using that buffer space will block. Note this makes the clients code tricky in what to do if it is blocked since one must ensure that a deadlock or starvation does not happen (but async messaging has always been tricky which is why cs community went with sync rpc). Note this problem does not arise on server side async-rpc since the client is blocked waiting for reply (unless the client also did async call but in that case its buffer, as per my suggestion, must be there to store the reply).
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          Steve Loughran why is the thread-local (i.e. asynchronousMode) not thread safe? The reason why thread-local is chosen is to make setting asynchronousMode in a thread safe way. If you look at HADOOP-12910 patch, e.g. FutureDistributedFileSystem#rename,

          62	      Client.setAsynchronousMode(true);
          63	      dfs.getClient().rename(dfs.getPathName(absSrc), dfs.getPathName(absDst),
          64	          options);
          65	      return Client.getReturnValue();
          

          After calling Client#rename, the Client#getReturnValue() is immediately called to get Future object used to retrieve final results. FutureDistributedFileSystem#rename should be always in the same thread with Client. There's no chance of interleaving sync and async to result in any thread safety issues.

          I agree that Client could implement AutoCloseable. Thanks.

          Show
          xiaobingo Xiaobing Zhou added a comment - Steve Loughran why is the thread-local (i.e. asynchronousMode) not thread safe? The reason why thread-local is chosen is to make setting asynchronousMode in a thread safe way. If you look at HADOOP-12910 patch, e.g. FutureDistributedFileSystem#rename, 62 Client.setAsynchronousMode( true ); 63 dfs.getClient().rename(dfs.getPathName(absSrc), dfs.getPathName(absDst), 64 options); 65 return Client.getReturnValue(); After calling Client#rename, the Client#getReturnValue() is immediately called to get Future object used to retrieve final results. FutureDistributedFileSystem#rename should be always in the same thread with Client. There's no chance of interleaving sync and async to result in any thread safety issues. I agree that Client could implement AutoCloseable. Thanks.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          ... I know it's not easy with some calls potentially being sync and some being async, but I'd propose having separate instances here. ...

          Are you saying that it needs two connections for supporting sync and async calls? It seems undesirable to have two connections. A single client should support both sync and async calls.

          ... Existing sync FS, through FileSystem.getFileSystem; the async one through FileSystem.getAsyncFilesystem() where the latter is (a) refcounted and (b) fully asyc

          I do agree with the point above but let's raise the FileSystem API discussion in HADOOP-12910 and focus on RPC API here.

          I do think it's ugly, and worry that it's dangerously easy to share a client across two threads, with consequences.

          The current implementation with ThreadLocal is thread safe, i.e. it is safe to share a client with two threads.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - ... I know it's not easy with some calls potentially being sync and some being async, but I'd propose having separate instances here. ... Are you saying that it needs two connections for supporting sync and async calls? It seems undesirable to have two connections. A single client should support both sync and async calls. ... Existing sync FS, through FileSystem.getFileSystem; the async one through FileSystem.getAsyncFilesystem() where the latter is (a) refcounted and (b) fully asyc I do agree with the point above but let's raise the FileSystem API discussion in HADOOP-12910 and focus on RPC API here. I do think it's ugly, and worry that it's dangerously easy to share a client across two threads, with consequences. The current implementation with ThreadLocal is thread safe, i.e. it is safe to share a client with two threads.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          BTW, the current patch aligns with the existing code – the existing callId and retryCount are ThreadLocal variables. The patch just adds two more ThreadLocal variables, asynchronousMode and returnValue.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - BTW, the current patch aligns with the existing code – the existing callId and retryCount are ThreadLocal variables. The patch just adds two more ThreadLocal variables, asynchronousMode and returnValue.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          Steve Loughran I am not clear for this when you said 'could this be set in the constructor?' Can you explain it? Thanks.

          LIke I said, Client#asynchronousMode is maintained through thread-local is thread safe. Anything other than that in Client is thread safe or thread not safe as is when it's been shared across threads.

          I will think of implementing FileSystem#getFileSystem and FileSystem#getAsyncFilesystem() over getting AsyncFilesystem through an instance of FileSystem (e.g. DistributedFileSystem), although in the latter case, there's benefit of easy implementation by reusing DistributedFileSystem object in async file system.

          Show
          xiaobingo Xiaobing Zhou added a comment - Steve Loughran I am not clear for this when you said 'could this be set in the constructor?' Can you explain it? Thanks. LIke I said, Client#asynchronousMode is maintained through thread-local is thread safe. Anything other than that in Client is thread safe or thread not safe as is when it's been shared across threads. I will think of implementing FileSystem#getFileSystem and FileSystem#getAsyncFilesystem() over getting AsyncFilesystem through an instance of FileSystem (e.g. DistributedFileSystem), although in the latter case, there's benefit of easy implementation by reusing DistributedFileSystem object in async file system.
          Hide
          wheat9 Haohui Mai added a comment -

          I'm concerned about the complexity and the effort to support it down the road.

          I understand in short term this is something that can be patched. However, we have enough troubles in today's IPC, maintaining it is definitely a challenging task especially. Does it make more sense to invest in other existing solutions that are more well tested, such as gRPC, which will allow us to make more effective investments in other areas?

          Show
          wheat9 Haohui Mai added a comment - I'm concerned about the complexity and the effort to support it down the road. I understand in short term this is something that can be patched. However, we have enough troubles in today's IPC, maintaining it is definitely a challenging task especially. Does it make more sense to invest in other existing solutions that are more well tested, such as gRPC, which will allow us to make more effective investments in other areas?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          well, there's google's track record in backwards compatibility of protobuf and guava libs to consider there...

          Show
          stevel@apache.org Steve Loughran added a comment - well, there's google's track record in backwards compatibility of protobuf and guava libs to consider there...
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          I posted patch V004 that fixed test/code issues Tsz Wo Nicholas Sze mentioned, thanks for this review.

          Show
          xiaobingo Xiaobing Zhou added a comment - I posted patch V004 that fixed test/code issues Tsz Wo Nicholas Sze mentioned, thanks for this review.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          Haohui Mai looks like gRPC is still in early stage, we can see the limited and predicable changes in current RPC to meet async case.

          Show
          xiaobingo Xiaobing Zhou added a comment - Haohui Mai looks like gRPC is still in early stage, we can see the limited and predicable changes in current RPC to meet async case.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 6m 55s trunk passed
          +1 compile 5m 49s trunk passed with JDK v1.8.0_74
          +1 compile 6m 29s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 22s trunk passed
          +1 mvnsite 0m 54s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 34s trunk passed
          +1 javadoc 0m 51s trunk passed with JDK v1.8.0_74
          +1 javadoc 1m 3s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 39s the patch passed
          +1 compile 5m 31s the patch passed with JDK v1.8.0_74
          -1 javac 9m 51s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738)
          +1 javac 5m 31s the patch passed
          +1 compile 6m 34s the patch passed with JDK v1.7.0_95
          -1 javac 16m 25s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 1 new + 733 unchanged - 1 fixed = 734 total (was 734)
          +1 javac 6m 34s the patch passed
          -1 checkstyle 0m 21s hadoop-common-project/hadoop-common: patch generated 5 new + 85 unchanged - 1 fixed = 90 total (was 86)
          +1 mvnsite 0m 55s the patch passed
          +1 mvneclipse 0m 14s the patch passed
          -1 whitespace 0m 0s The patch has 19 line(s) that end in whitespace. Use git apply --whitespace=fix.
          -1 whitespace 0m 0s The patch has 1 line(s) with tabs.
          +1 findbugs 1m 48s the patch passed
          +1 javadoc 0m 49s the patch passed with JDK v1.8.0_74
          +1 javadoc 1m 2s the patch passed with JDK v1.7.0_95
          -1 unit 6m 32s hadoop-common in the patch failed with JDK v1.8.0_74.
          -1 unit 6m 51s hadoop-common in the patch failed with JDK v1.7.0_95.
          -1 asflicense 0m 23s Patch generated 2 ASF License warnings.
          57m 8s



          Reason Tests
          JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker
          JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12794903/HADOOP-12909-HDFS-9924.004.patch
          JIRA Issue HADOOP-12909
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux cf6f8c534979 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 0bfe5a0
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt
          javac root-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/whitespace-eol.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/whitespace-tabs.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 6m 55s trunk passed +1 compile 5m 49s trunk passed with JDK v1.8.0_74 +1 compile 6m 29s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 54s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 34s trunk passed +1 javadoc 0m 51s trunk passed with JDK v1.8.0_74 +1 javadoc 1m 3s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 39s the patch passed +1 compile 5m 31s the patch passed with JDK v1.8.0_74 -1 javac 9m 51s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738) +1 javac 5m 31s the patch passed +1 compile 6m 34s the patch passed with JDK v1.7.0_95 -1 javac 16m 25s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 1 new + 733 unchanged - 1 fixed = 734 total (was 734) +1 javac 6m 34s the patch passed -1 checkstyle 0m 21s hadoop-common-project/hadoop-common: patch generated 5 new + 85 unchanged - 1 fixed = 90 total (was 86) +1 mvnsite 0m 55s the patch passed +1 mvneclipse 0m 14s the patch passed -1 whitespace 0m 0s The patch has 19 line(s) that end in whitespace. Use git apply --whitespace=fix. -1 whitespace 0m 0s The patch has 1 line(s) with tabs. +1 findbugs 1m 48s the patch passed +1 javadoc 0m 49s the patch passed with JDK v1.8.0_74 +1 javadoc 1m 2s the patch passed with JDK v1.7.0_95 -1 unit 6m 32s hadoop-common in the patch failed with JDK v1.8.0_74. -1 unit 6m 51s hadoop-common in the patch failed with JDK v1.7.0_95. -1 asflicense 0m 23s Patch generated 2 ASF License warnings. 57m 8s Reason Tests JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12794903/HADOOP-12909-HDFS-9924.004.patch JIRA Issue HADOOP-12909 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux cf6f8c534979 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 0bfe5a0 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt javac root-jdk1.7.0_95: https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/whitespace-eol.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/whitespace-tabs.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/testReport/ asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8903/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I was proposing having separate sync and async clients, fixed at construction

          Show
          stevel@apache.org Steve Loughran added a comment - I was proposing having separate sync and async clients, fixed at construction
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          Thanks Sanjay Radia for the comment. To avoid buffer use-up issue, we can limit the number of outstanding async calls, let's say up to 100, otherwise caller will be blocked.

          Show
          xiaobingo Xiaobing Zhou added a comment - Thanks Sanjay Radia for the comment. To avoid buffer use-up issue, we can limit the number of outstanding async calls, let's say up to 100, otherwise caller will be blocked.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          I was proposing having separate sync and async clients, fixed at construction

          Thanks Steve Loughran for clarifying it. Minor changes now in Client support both sync and async cases. Is it still necessary to have separate clients?

          Show
          xiaobingo Xiaobing Zhou added a comment - I was proposing having separate sync and async clients, fixed at construction Thanks Steve Loughran for clarifying it. Minor changes now in Client support both sync and async cases. Is it still necessary to have separate clients?
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          V005 fixed check style issues, and had Client implement AutoCloseable.

          Show
          xiaobingo Xiaobing Zhou added a comment - V005 fixed check style issues, and had Client implement AutoCloseable.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 9m 32s trunk passed
          +1 compile 12m 0s trunk passed with JDK v1.8.0_74
          +1 compile 10m 22s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 32s trunk passed
          +1 mvnsite 1m 20s trunk passed
          +1 mvneclipse 0m 19s trunk passed
          +1 findbugs 2m 9s trunk passed
          +1 javadoc 1m 24s trunk passed with JDK v1.8.0_74
          +1 javadoc 1m 31s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 58s the patch passed
          +1 compile 12m 0s the patch passed with JDK v1.8.0_74
          -1 javac 18m 21s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738)
          +1 javac 12m 0s the patch passed
          +1 compile 10m 19s the patch passed with JDK v1.7.0_95
          +1 javac 10m 19s the patch passed
          -1 checkstyle 0m 31s hadoop-common-project/hadoop-common: patch generated 2 new + 85 unchanged - 1 fixed = 87 total (was 86)
          +1 mvnsite 1m 19s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 2m 29s the patch passed
          +1 javadoc 1m 25s the patch passed with JDK v1.8.0_74
          +1 javadoc 1m 33s the patch passed with JDK v1.7.0_95
          -1 unit 11m 23s hadoop-common in the patch failed with JDK v1.8.0_74.
          -1 unit 10m 36s hadoop-common in the patch failed with JDK v1.7.0_95.
          -1 asflicense 0m 33s Patch generated 2 ASF License warnings.
          94m 48s



          Reason Tests
          JDK v1.8.0_74 Failed junit tests hadoop.ipc.TestRPCWaitForProxy
            hadoop.fs.shell.find.TestPrint0
            hadoop.fs.shell.find.TestPrint
          JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker
          JDK v1.7.0_95 Failed junit tests hadoop.ipc.TestRPCWaitForProxy
          JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12795092/HADOOP-12909-HDFS-9924.005.patch
          JIRA Issue HADOOP-12909
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c7509e498d69 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 938222b
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 9m 32s trunk passed +1 compile 12m 0s trunk passed with JDK v1.8.0_74 +1 compile 10m 22s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 32s trunk passed +1 mvnsite 1m 20s trunk passed +1 mvneclipse 0m 19s trunk passed +1 findbugs 2m 9s trunk passed +1 javadoc 1m 24s trunk passed with JDK v1.8.0_74 +1 javadoc 1m 31s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 58s the patch passed +1 compile 12m 0s the patch passed with JDK v1.8.0_74 -1 javac 18m 21s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738) +1 javac 12m 0s the patch passed +1 compile 10m 19s the patch passed with JDK v1.7.0_95 +1 javac 10m 19s the patch passed -1 checkstyle 0m 31s hadoop-common-project/hadoop-common: patch generated 2 new + 85 unchanged - 1 fixed = 87 total (was 86) +1 mvnsite 1m 19s the patch passed +1 mvneclipse 0m 21s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 2m 29s the patch passed +1 javadoc 1m 25s the patch passed with JDK v1.8.0_74 +1 javadoc 1m 33s the patch passed with JDK v1.7.0_95 -1 unit 11m 23s hadoop-common in the patch failed with JDK v1.8.0_74. -1 unit 10m 36s hadoop-common in the patch failed with JDK v1.7.0_95. -1 asflicense 0m 33s Patch generated 2 ASF License warnings. 94m 48s Reason Tests JDK v1.8.0_74 Failed junit tests hadoop.ipc.TestRPCWaitForProxy   hadoop.fs.shell.find.TestPrint0   hadoop.fs.shell.find.TestPrint JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker JDK v1.7.0_95 Failed junit tests hadoop.ipc.TestRPCWaitForProxy JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12795092/HADOOP-12909-HDFS-9924.005.patch JIRA Issue HADOOP-12909 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c7509e498d69 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 938222b Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/testReport/ asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8908/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I'll let sanjay handle the architectural issues.

          Production code

          Regarding the client setup, it's still something I worry about, but it is workable. I'd recommend those methods are marked as Unstable. Actually, there aren't any visibility tags on Client and Server at all: I'd recommend @Public, @Evolving for the whole class, as well as @Unstable for the new methods.
          In Client.call(RPC.RpcKind rpcKind, Writable rpcRequest)

          line 1370; warning text includes detail not added to the exception. Can you assign that extra text to a string and included

          String text =  String.format("Error in instantiating Writable %s", rpcRequest.getClass());
          LOG.warn(text, ie)
          throw new IOException(text + " " + ie, ie);
          

          Tests.

          • If TestIPC.TestServer was also made AutoCloseable, with close() calling stop(), you could use try-with-resources to do all the test cleanup; potentially make for cleanup/small test cases.
          • A couple of your assertEquals clauses are the wrong way round; expected must always come first.
          • I'd like some text string of failure causes in the assertFalse() calls. Imagine the test has failed in a Jenkins run: what extra information would you need to start debugging the failure.
          Show
          stevel@apache.org Steve Loughran added a comment - I'll let sanjay handle the architectural issues. Production code Regarding the client setup, it's still something I worry about, but it is workable. I'd recommend those methods are marked as Unstable. Actually, there aren't any visibility tags on Client and Server at all: I'd recommend @Public, @Evolving for the whole class, as well as @Unstable for the new methods. In Client.call(RPC.RpcKind rpcKind, Writable rpcRequest) line 1370; warning text includes detail not added to the exception. Can you assign that extra text to a string and included String text = String .format( "Error in instantiating Writable %s" , rpcRequest.getClass()); LOG.warn(text, ie) throw new IOException(text + " " + ie, ie); Tests. If TestIPC.TestServer was also made AutoCloseable , with close() calling stop() , you could use try-with-resources to do all the test cleanup; potentially make for cleanup/small test cases. A couple of your assertEquals clauses are the wrong way round; expected must always come first. I'd like some text string of failure causes in the assertFalse() calls. Imagine the test has failed in a Jenkins run: what extra information would you need to start debugging the failure.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          I posted v006 to fix your comments thanks Steve Loughran. I filed HADOOP-12968 to address changes of TestIPC.TestServer. I also filed HADOOP-12969 to cover marking IPC.Client and IPC.Server as @Public, @Evolving.

          Show
          xiaobingo Xiaobing Zhou added a comment - I posted v006 to fix your comments thanks Steve Loughran . I filed HADOOP-12968 to address changes of TestIPC.TestServer. I also filed HADOOP-12969 to cover marking IPC.Client and IPC.Server as @Public, @Evolving.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          OK, let Sanjay comment; I have no objections to the code as is

          Show
          stevel@apache.org Steve Loughran added a comment - OK, let Sanjay comment; I have no objections to the code as is
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -
          • The parameter type in setAsynchronousMode(Boolean async) should be the lower case boolean.
          • getRpcResponse(..) is private. Let's remove @Unstable.
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - The parameter type in setAsynchronousMode(Boolean async) should be the lower case boolean. getRpcResponse(..) is private. Let's remove @Unstable.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          Thanks Tsz Wo Nicholas Sze, v007 removed @Unstable and changed it to lowercase one.

          Show
          xiaobingo Xiaobing Zhou added a comment - Thanks Tsz Wo Nicholas Sze , v007 removed @Unstable and changed it to lowercase one.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -
          • It seems that we could simply return null instead of a dummy value in Client.call(..).
          • The comments/javadoc should not mention HDFS. This change is in Common only.
          • The setAsynchronousMode() method in TestAsyncIPC can be removed.
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - It seems that we could simply return null instead of a dummy value in Client.call(..). The comments/javadoc should not mention HDFS. This change is in Common only. The setAsynchronousMode() method in TestAsyncIPC can be removed.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          Some more comments for TestAsyncIPC:

          • TestAsyncIPC seems not really testing asynchronous since it always immediately calls returnFuture.get() to block the caller. Could you add some new tests to test asynchronous?
          • TestAsyncIPC should not extends TestIPC. It makes the test complicated. We may change the code in TestIPC package private instead of protected (i.e. just remove "private" but not adding "protected"). Then TestAsyncIPC can reuse the code in TestIPC.
          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - Some more comments for TestAsyncIPC: TestAsyncIPC seems not really testing asynchronous since it always immediately calls returnFuture.get() to block the caller. Could you add some new tests to test asynchronous? TestAsyncIPC should not extends TestIPC. It makes the test complicated. We may change the code in TestIPC package private instead of protected (i.e. just remove "private" but not adding "protected"). Then TestAsyncIPC can reuse the code in TestIPC.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          Patch v008 addressed all your latest comments except adding new tests. I will add them in next patch. The benefit of TestAsyncIPC extending TestIPC is to reuse most of IPC test logic and make them working correctly for both cases of sync and async. Those that can't be reused go to TestAsyncIPC, e.g. SerialCaller. TestIPC keeps stable as is. Thanks Tsz Wo Nicholas Sze for review.

          Show
          xiaobingo Xiaobing Zhou added a comment - Patch v008 addressed all your latest comments except adding new tests. I will add them in next patch. The benefit of TestAsyncIPC extending TestIPC is to reuse most of IPC test logic and make them working correctly for both cases of sync and async. Those that can't be reused go to TestAsyncIPC, e.g. SerialCaller. TestIPC keeps stable as is. Thanks Tsz Wo Nicholas Sze for review.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 6m 41s trunk passed
          +1 compile 6m 2s trunk passed with JDK v1.8.0_74
          +1 compile 6m 45s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 22s trunk passed
          +1 mvnsite 0m 56s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 32s trunk passed
          +1 javadoc 0m 56s trunk passed with JDK v1.8.0_74
          +1 javadoc 1m 3s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 41s the patch passed
          +1 compile 6m 6s the patch passed with JDK v1.8.0_74
          -1 javac 10m 30s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738)
          +1 javac 6m 6s the patch passed
          +1 compile 6m 50s the patch passed with JDK v1.7.0_95
          +1 javac 6m 50s the patch passed
          -1 checkstyle 0m 22s hadoop-common-project/hadoop-common: patch generated 2 new + 85 unchanged - 1 fixed = 87 total (was 86)
          +1 mvnsite 0m 58s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 1m 49s the patch passed
          +1 javadoc 0m 54s the patch passed with JDK v1.8.0_74
          +1 javadoc 1m 3s the patch passed with JDK v1.7.0_95
          -1 unit 7m 3s hadoop-common in the patch failed with JDK v1.8.0_74.
          -1 unit 7m 22s hadoop-common in the patch failed with JDK v1.7.0_95.
          -1 asflicense 0m 22s Patch generated 2 ASF License warnings.
          59m 38s



          Reason Tests
          JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker
          JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12796363/HADOOP-12909-HDFS-9924.008.patch
          JIRA Issue HADOOP-12909
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 756ce7ed6bf9 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 6d67420
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 6m 41s trunk passed +1 compile 6m 2s trunk passed with JDK v1.8.0_74 +1 compile 6m 45s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 56s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 32s trunk passed +1 javadoc 0m 56s trunk passed with JDK v1.8.0_74 +1 javadoc 1m 3s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 41s the patch passed +1 compile 6m 6s the patch passed with JDK v1.8.0_74 -1 javac 10m 30s root-jdk1.8.0_74 with JDK v1.8.0_74 generated 1 new + 737 unchanged - 1 fixed = 738 total (was 738) +1 javac 6m 6s the patch passed +1 compile 6m 50s the patch passed with JDK v1.7.0_95 +1 javac 6m 50s the patch passed -1 checkstyle 0m 22s hadoop-common-project/hadoop-common: patch generated 2 new + 85 unchanged - 1 fixed = 87 total (was 86) +1 mvnsite 0m 58s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 1m 49s the patch passed +1 javadoc 0m 54s the patch passed with JDK v1.8.0_74 +1 javadoc 1m 3s the patch passed with JDK v1.7.0_95 -1 unit 7m 3s hadoop-common in the patch failed with JDK v1.8.0_74. -1 unit 7m 22s hadoop-common in the patch failed with JDK v1.7.0_95. -1 asflicense 0m 22s Patch generated 2 ASF License warnings. 59m 38s Reason Tests JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12796363/HADOOP-12909-HDFS-9924.008.patch JIRA Issue HADOOP-12909 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 756ce7ed6bf9 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6d67420 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_74 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac root-jdk1.8.0_74: https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_74.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/testReport/ asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8992/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          > ... The benefit of TestAsyncIPC extending TestIPC is to reuse most of IPC test logic and make them working correctly for both cases of sync and async. ...

          As mentioned previously, TestAsyncIPC is not really testing asynchronous calls. It is testing synchronous calls in asynchronous mode (by immediately calling Future.get() after each async call). Should we reuse the logic in TestIPC, which is designed for testing synchronous calls? In other words, are the current tests in TestAsyncIPC really relevant to the new asynchronous call feature?

          BTW, for reusing the test logic, we may change the private methods/classes in TestIPC to package private and change them to static so that TestAsyncIPC can use them. When TestAsyncIPC extends TestIPC, it is very hard to understand how does TestAsyncIPC work.

          I do suggest we focus on testing asynchronous calls but not synchronous calls in asynchronous mode. We may drop the the current tests in TestAsyncIPC instead of spending time on fixing them.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - > ... The benefit of TestAsyncIPC extending TestIPC is to reuse most of IPC test logic and make them working correctly for both cases of sync and async. ... As mentioned previously, TestAsyncIPC is not really testing asynchronous calls. It is testing synchronous calls in asynchronous mode (by immediately calling Future.get() after each async call). Should we reuse the logic in TestIPC, which is designed for testing synchronous calls? In other words, are the current tests in TestAsyncIPC really relevant to the new asynchronous call feature? BTW, for reusing the test logic, we may change the private methods/classes in TestIPC to package private and change them to static so that TestAsyncIPC can use them. When TestAsyncIPC extends TestIPC, it is very hard to understand how does TestAsyncIPC work. I do suggest we focus on testing asynchronous calls but not synchronous calls in asynchronous mode. We may drop the the current tests in TestAsyncIPC instead of spending time on fixing them.
          Hide
          xiaobingo Xiaobing Zhou added a comment -

          v009 is posted by adding tests for asynchronous case since the previous versions tested synchronous calls in asynchronous mode. In addition, TestAsyncIPC doesn't extend TestIPC anymore. Thank you for review Tsz Wo Nicholas Sze.

          Show
          xiaobingo Xiaobing Zhou added a comment - v009 is posted by adding tests for asynchronous case since the previous versions tested synchronous calls in asynchronous mode. In addition, TestAsyncIPC doesn't extend TestIPC anymore. Thank you for review Tsz Wo Nicholas Sze .
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 9s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          +1 mvninstall 6m 41s trunk passed
          +1 compile 5m 55s trunk passed with JDK v1.8.0_77
          +1 compile 6m 46s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 22s trunk passed
          +1 mvnsite 0m 57s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 1m 36s trunk passed
          +1 javadoc 0m 55s trunk passed with JDK v1.8.0_77
          +1 javadoc 1m 3s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 40s the patch passed
          +1 compile 5m 50s the patch passed with JDK v1.8.0_77
          +1 javac 5m 50s the patch passed
          +1 compile 6m 41s the patch passed with JDK v1.7.0_95
          +1 javac 6m 41s the patch passed
          -1 checkstyle 0m 22s hadoop-common-project/hadoop-common: patch generated 2 new + 85 unchanged - 1 fixed = 87 total (was 86)
          +1 mvnsite 0m 55s the patch passed
          +1 mvneclipse 0m 15s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 1m 50s the patch passed
          +1 javadoc 0m 54s the patch passed with JDK v1.8.0_77
          +1 javadoc 1m 4s the patch passed with JDK v1.7.0_95
          +1 unit 7m 19s hadoop-common in the patch passed with JDK v1.8.0_77.
          +1 unit 7m 40s hadoop-common in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 22s Patch does not generate ASF License warnings.
          59m 36s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12797194/HADOOP-12909-HDFS-9924.009.patch
          JIRA Issue HADOOP-12909
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f4e47f141f95 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 4bd7cbc
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9037/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9037/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9037/console
          Powered by Apache Yetus 0.2.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 9s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. +1 mvninstall 6m 41s trunk passed +1 compile 5m 55s trunk passed with JDK v1.8.0_77 +1 compile 6m 46s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 57s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 1m 36s trunk passed +1 javadoc 0m 55s trunk passed with JDK v1.8.0_77 +1 javadoc 1m 3s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 40s the patch passed +1 compile 5m 50s the patch passed with JDK v1.8.0_77 +1 javac 5m 50s the patch passed +1 compile 6m 41s the patch passed with JDK v1.7.0_95 +1 javac 6m 41s the patch passed -1 checkstyle 0m 22s hadoop-common-project/hadoop-common: patch generated 2 new + 85 unchanged - 1 fixed = 87 total (was 86) +1 mvnsite 0m 55s the patch passed +1 mvneclipse 0m 15s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 1m 50s the patch passed +1 javadoc 0m 54s the patch passed with JDK v1.8.0_77 +1 javadoc 1m 4s the patch passed with JDK v1.7.0_95 +1 unit 7m 19s hadoop-common in the patch passed with JDK v1.8.0_77. +1 unit 7m 40s hadoop-common in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 22s Patch does not generate ASF License warnings. 59m 36s Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12797194/HADOOP-12909-HDFS-9924.009.patch JIRA Issue HADOOP-12909 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f4e47f141f95 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 4bd7cbc Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_77 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9037/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9037/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9037/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          +1 the new patch looks great.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - +1 the new patch looks great.
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9575 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9575/)
          HADOOP-12909. Change ipc.Client to support asynchronous calls. (szetszwo: rev a62637a413ad88c4273d3251892b8fc1c05afa34)

          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestAsyncIPC.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9575 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9575/ ) HADOOP-12909 . Change ipc.Client to support asynchronous calls. (szetszwo: rev a62637a413ad88c4273d3251892b8fc1c05afa34) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestAsyncIPC.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment - - edited

          > ... Actually, there aren't any visibility tags on Client and Server at all: I'd recommend @Public, @Evolving for the whole class ...

          Indeed, Client and Server are currently annotated as @InterfaceAudience.LimitedPrivate and @InterfaceStability.Evolving by HADOOP-8813. I think we don't need to change them to public. Let's continue the discussion in HADOOP-12969.

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - - edited > ... Actually, there aren't any visibility tags on Client and Server at all: I'd recommend @Public, @Evolving for the whole class ... Indeed, Client and Server are currently annotated as @InterfaceAudience.LimitedPrivate and @InterfaceStability.Evolving by HADOOP-8813 . I think we don't need to change them to public. Let's continue the discussion in HADOOP-12969 .
          Hide
          szetszwo Tsz Wo Nicholas Sze added a comment -

          I have committed this. Thanks, Xiaobing!

          Show
          szetszwo Tsz Wo Nicholas Sze added a comment - I have committed this. Thanks, Xiaobing!

            People

            • Assignee:
              xiaobingo Xiaobing Zhou
              Reporter:
              szetszwo Tsz Wo Nicholas Sze
            • Votes:
              0 Vote for this issue
              Watchers:
              22 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development