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

Remove synchronized input streams from Writable deserialization

    Details

    • Hadoop Flags:
      Reviewed
    • Release Note:
      Remove invisible synchronization primitives from DataInputBuffer

      Description

      Writable deserialization is slowing down due to a synchronized block within DataInputBuffer$Buffer.

      ByteArrayInputStream::read() is synchronized and this shows up as a slow uncontested lock.

      Hive ships with its own faster thread-unsafe version with hive.common.io.NonSyncByteArrayInputStream.

      The DataInputBuffer and Writable deserialization should not require a lock per readInt()/read().

      1. HADOOP-10694.1.patch
        2 kB
        Gopal V
      2. HADOOP-10694.2.patch
        3 kB
        Rajesh Balamohan
      3. writable-read-sync.png
        255 kB
        Gopal V

        Issue Links

          Activity

          Hide
          gopalv Gopal V added a comment -

          DataInputBuffer with unsynchronized inputstream code from hive's NonSyncByteArrayInputStream.

          Show
          gopalv Gopal V added a comment - DataInputBuffer with unsynchronized inputstream code from hive's NonSyncByteArrayInputStream.
          Hide
          gopalv Gopal V added a comment -

          SortBuffer does its own synchronization with its spillLock implementation.

          This particular synchronized block (which is invisible & inherited) will always only be called from one thread because of the locked spill/collect thread.

          Show
          gopalv Gopal V added a comment - SortBuffer does its own synchronization with its spillLock implementation. This particular synchronized block (which is invisible & inherited) will always only be called from one thread because of the locked spill/collect thread.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 14m 40s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 javac 7m 30s There were no new javac warning messages.
          +1 javadoc 9m 38s There were no new javadoc warning messages.
          +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 1m 6s The applied patch generated 2 new checkstyle issues (total was 5, now 7).
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 31s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 1m 39s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
          +1 common tests 22m 20s Tests passed in hadoop-common.
              59m 22s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12650437/HADOOP-10694.1.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 6ae2a0d
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/6432/artifact/patchprocess/diffcheckstylehadoop-common.txt
          hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6432/artifact/patchprocess/testrun_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6432/testReport/
          Java 1.7.0_55
          uname Linux asf900.gq1.ygridcore.net 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
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6432/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 14m 40s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 7m 30s There were no new javac warning messages. +1 javadoc 9m 38s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 1m 6s The applied patch generated 2 new checkstyle issues (total was 5, now 7). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 31s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 1m 39s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 common tests 22m 20s Tests passed in hadoop-common.     59m 22s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12650437/HADOOP-10694.1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 6ae2a0d checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/6432/artifact/patchprocess/diffcheckstylehadoop-common.txt hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/6432/artifact/patchprocess/testrun_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/6432/testReport/ Java 1.7.0_55 uname Linux asf900.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/6432/console This message was automatically generated.
          Hide
          hadoopqa Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          0 pre-patch 16m 29s Pre-patch trunk compilation is healthy.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 javac 7m 39s There were no new javac warning messages.
          +1 javadoc 9m 43s There were no new javadoc warning messages.
          +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
          -1 checkstyle 1m 4s The applied patch generated 2 new checkstyle issues (total was 5, now 7).
          +1 whitespace 0m 0s The patch has no lines that end in whitespace.
          +1 install 1m 18s mvn install still works.
          +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
          +1 findbugs 1m 50s The patch does not introduce any new Findbugs (version 3.0.0) warnings.
          +1 common tests 22m 0s Tests passed in hadoop-common.
              61m 1s  



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12650437/HADOOP-10694.1.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 2e3d83f
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/7202/artifact/patchprocess/diffcheckstylehadoop-common.txt
          hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7202/artifact/patchprocess/testrun_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7202/testReport/
          Java 1.7.0_55
          uname Linux asf901.gq1.ygridcore.net 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
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7202/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 16m 29s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. -1 tests included 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javac 7m 39s There were no new javac warning messages. +1 javadoc 9m 43s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 1m 4s The applied patch generated 2 new checkstyle issues (total was 5, now 7). +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 18s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 1m 50s The patch does not introduce any new Findbugs (version 3.0.0) warnings. +1 common tests 22m 0s Tests passed in hadoop-common.     61m 1s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12650437/HADOOP-10694.1.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 2e3d83f checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/7202/artifact/patchprocess/diffcheckstylehadoop-common.txt hadoop-common test log https://builds.apache.org/job/PreCommit-HADOOP-Build/7202/artifact/patchprocess/testrun_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/7202/testReport/ Java 1.7.0_55 uname Linux asf901.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7202/console This message was automatically generated.
          Hide
          ozawa Tsuyoshi Ozawa added a comment -

          Gopal V thank you for the contribution. +1 for the change.

          However, it's a bit dangerous to remove synchronization from DataInputBuffer because there are lots caller of DataInputBuffer. How about adding NonSyncByteArrayInputStream and changing to use it in MapTask and ReduceTask instead of removing lock?

          Additionaly, I think that it would be better NonSyncByteArrayInputStream's extending DataInputBuffer, because we don't need to change other code except new statement of NonSyncByteArrayInputStream instead of DataInputBuffer.

          Show
          ozawa Tsuyoshi Ozawa added a comment - Gopal V thank you for the contribution. +1 for the change. However, it's a bit dangerous to remove synchronization from DataInputBuffer because there are lots caller of DataInputBuffer. How about adding NonSyncByteArrayInputStream and changing to use it in MapTask and ReduceTask instead of removing lock? Additionaly, I think that it would be better NonSyncByteArrayInputStream's extending DataInputBuffer, because we don't need to change other code except new statement of NonSyncByteArrayInputStream instead of DataInputBuffer.
          Hide
          ozawa Tsuyoshi Ozawa added a comment -

          Canceling the patch for the comment.

          Show
          ozawa Tsuyoshi Ozawa added a comment - Canceling the patch for the comment.
          Hide
          gopalv Gopal V added a comment -

          Tsuyoshi Ozawa: I think the synchronization is unintentional, since two threads using DataInputBuffer readers without synchronization would cause corruption (something like readInt() would result in interleaved corruption, because each byte read() is independently synchronized).

          Show
          gopalv Gopal V added a comment - Tsuyoshi Ozawa : I think the synchronization is unintentional, since two threads using DataInputBuffer readers without synchronization would cause corruption (something like readInt() would result in interleaved corruption, because each byte read() is independently synchronized).
          Hide
          ozawa Tsuyoshi Ozawa added a comment -

          Gopal V Make sense. Could you fix checkstyle issue? Additionally, I think it's a good timing to fix asterisk import(
          import java.io.*) though it's not related to the issue directly. I'm +1 if the comment is addressed.

          Show
          ozawa Tsuyoshi Ozawa added a comment - Gopal V Make sense. Could you fix checkstyle issue? Additionally, I think it's a good timing to fix asterisk import( import java.io.*) though it's not related to the issue directly. I'm +1 if the comment is addressed.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 28s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 mvninstall 10m 11s trunk passed
          +1 compile 12m 2s trunk passed with JDK v1.8.0_92
          +1 compile 9m 42s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 26s trunk passed
          +1 mvnsite 1m 23s trunk passed
          +1 mvneclipse 0m 19s trunk passed
          +1 findbugs 2m 1s trunk passed
          +1 javadoc 1m 24s trunk passed with JDK v1.8.0_92
          +1 javadoc 1m 32s trunk passed with JDK v1.7.0_95
          +1 mvninstall 1m 0s the patch passed
          +1 compile 11m 55s the patch passed with JDK v1.8.0_92
          +1 javac 11m 55s the patch passed
          +1 compile 10m 12s the patch passed with JDK v1.7.0_95
          +1 javac 10m 12s the patch passed
          +1 checkstyle 0m 26s hadoop-common-project/hadoop-common: The patch generated 0 new + 2 unchanged - 3 fixed = 2 total (was 5)
          +1 mvnsite 1m 20s the patch passed
          +1 mvneclipse 0m 18s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 23s the patch passed
          +1 javadoc 1m 32s the patch passed with JDK v1.8.0_92
          +1 javadoc 1m 32s the patch passed with JDK v1.7.0_95
          -1 unit 10m 18s hadoop-common in the patch failed with JDK v1.8.0_92.
          -1 unit 9m 38s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 24s The patch does not generate ASF License warnings.
          92m 1s



          Reason Tests
          JDK v1.8.0_92 Failed junit tests hadoop.net.TestDNS
          JDK v1.7.0_95 Failed junit tests hadoop.security.ssl.TestReloadingX509TrustManager



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801942/HADOOP-10694.2.patch
          JIRA Issue HADOOP-10694
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7ae861852d66 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 / 06413da
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_92 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9258/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_92.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9258/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/9258/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_92.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9258/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/9258/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9258/console
          Powered by Apache Yetus 0.3.0-SNAPSHOT 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 28s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 10m 11s trunk passed +1 compile 12m 2s trunk passed with JDK v1.8.0_92 +1 compile 9m 42s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 26s trunk passed +1 mvnsite 1m 23s trunk passed +1 mvneclipse 0m 19s trunk passed +1 findbugs 2m 1s trunk passed +1 javadoc 1m 24s trunk passed with JDK v1.8.0_92 +1 javadoc 1m 32s trunk passed with JDK v1.7.0_95 +1 mvninstall 1m 0s the patch passed +1 compile 11m 55s the patch passed with JDK v1.8.0_92 +1 javac 11m 55s the patch passed +1 compile 10m 12s the patch passed with JDK v1.7.0_95 +1 javac 10m 12s the patch passed +1 checkstyle 0m 26s hadoop-common-project/hadoop-common: The patch generated 0 new + 2 unchanged - 3 fixed = 2 total (was 5) +1 mvnsite 1m 20s the patch passed +1 mvneclipse 0m 18s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 23s the patch passed +1 javadoc 1m 32s the patch passed with JDK v1.8.0_92 +1 javadoc 1m 32s the patch passed with JDK v1.7.0_95 -1 unit 10m 18s hadoop-common in the patch failed with JDK v1.8.0_92. -1 unit 9m 38s hadoop-common in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 24s The patch does not generate ASF License warnings. 92m 1s Reason Tests JDK v1.8.0_92 Failed junit tests hadoop.net.TestDNS JDK v1.7.0_95 Failed junit tests hadoop.security.ssl.TestReloadingX509TrustManager Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801942/HADOOP-10694.2.patch JIRA Issue HADOOP-10694 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7ae861852d66 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 / 06413da Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_92 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9258/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_92.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9258/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/9258/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_92.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9258/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/9258/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9258/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          ozawa Tsuyoshi Ozawa added a comment -

          +1, checking this in.

          Show
          ozawa Tsuyoshi Ozawa added a comment - +1, checking this in.
          Hide
          ozawa Tsuyoshi Ozawa added a comment -

          The test failures looks to be not related since TestDNS and TestReloadingX509TrustManager fails by DNS related problem.

          Show
          ozawa Tsuyoshi Ozawa added a comment - The test failures looks to be not related since TestDNS and TestReloadingX509TrustManager fails by DNS related problem.
          Hide
          ozawa Tsuyoshi Ozawa added a comment -

          Committed this to trunk and branch-2. Thanks Gopal V and Rajesh Balamohan for your contribution!

          Show
          ozawa Tsuyoshi Ozawa added a comment - Committed this to trunk and branch-2. Thanks Gopal V and Rajesh Balamohan for your contribution!
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9743 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9743/)
          HADOOP-10694. Remove synchronized input streams from Writable (ozawa: rev 6e565780315469584c47515be6bd189f07840f1b)

          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/DataInputBuffer.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9743 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9743/ ) HADOOP-10694 . Remove synchronized input streams from Writable (ozawa: rev 6e565780315469584c47515be6bd189f07840f1b) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/DataInputBuffer.java

            People

            • Assignee:
              gopalv Gopal V
              Reporter:
              gopalv Gopal V
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development