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

Add a new interface for retrieving FS and FC Statistics

    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: fs
    • Labels:
      None
    • Target Version/s:

      Description

      Currently FileSystem.Statistics exposes the following statistics:
      BytesRead
      BytesWritten
      ReadOps
      LargeReadOps
      WriteOps

      These are in-turn exposed as job counters by MapReduce and other frameworks. There is logic within DfsClient to map operations to these counters that can be confusing, for instance, mkdirs counts as a writeOp.

      Proposed enhancement:
      Add a statistic for each DfsClient operation including create, append, createSymlink, delete, exists, mkdirs, rename and expose them as new properties on the Statistics object. The operation-specific counters can be used for analyzing the load imposed by a particular job on HDFS.
      For example, we can use them to identify jobs that end up creating a large number of files.

      Once this information is available in the Statistics object, the app frameworks like MapReduce can expose them as additional counters to be aggregated and recorded as part of job summary.

      1. HADOOP-13065.013.patch
        79 kB
        Mingliang Liu
      2. HADOOP-13065.012.patch
        79 kB
        Mingliang Liu
      3. HADOOP-13065.011.patch
        79 kB
        Mingliang Liu
      4. HADOOP-13065.010.patch
        77 kB
        Mingliang Liu
      5. HADOOP-13065.009.patch
        76 kB
        Mingliang Liu
      6. HADOOP-13065.008.patch
        73 kB
        Mingliang Liu
      7. HADOOP-13065-007.patch
        20 kB
        Colin P. McCabe
      8. HDFS-10175.006.patch
        8 kB
        Colin P. McCabe
      9. HDFS-10175.005.patch
        53 kB
        Mingliang Liu
      10. HDFS-10175.004.patch
        49 kB
        Mingliang Liu
      11. TestStatisticsOverhead.java
        3 kB
        Colin P. McCabe
      12. HDFS-10175.003.patch
        49 kB
        Mingliang Liu
      13. HDFS-10175.002.patch
        47 kB
        Mingliang Liu
      14. HDFS-10175.001.patch
        48 kB
        Mingliang Liu
      15. HDFS-10175.000.patch
        51 kB
        Mingliang Liu

        Issue Links

          Activity

          Hide
          fabbri Aaron Fabbri added a comment -

          Ooops.. Not sure how I accidentally clicked Assign To Me.. thanks for fixing that Hitesh Shah

          Show
          fabbri Aaron Fabbri added a comment - Ooops.. Not sure how I accidentally clicked Assign To Me.. thanks for fixing that Hitesh Shah
          Hide
          mingma Ming Ma added a comment -

          Mingliang Liu Colin P. McCabe, yeah the perf could be an issue. Regarding moving it to ReadStatistics, the problem is there is no easy way for MR framework to get hold of the HDFS streams used by the application.

          Show
          mingma Ming Ma added a comment - Mingliang Liu Colin P. McCabe , yeah the perf could be an issue. Regarding moving it to ReadStatistics, the problem is there is no easy way for MR framework to get hold of the HDFS streams used by the application.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Looks like this is breaking Jenkins in HDFS-10418; can someone take a look at it. thanks

          Show
          stevel@apache.org Steve Loughran added a comment - Looks like this is breaking Jenkins in HDFS-10418 ; can someone take a look at it. thanks
          Hide
          cmccabe Colin P. McCabe added a comment -

          I filed HADOOP-13140 to track the effort.

          Thanks, Mingliang Liu.

          Ming Ma wrote: BTW, is the network distance metrics something general for all file system or it is more specific to HDFS? For example, local file system doesn't need that. If it is more HDFS specific, wonder if we should move it to HDFS specific metrics.

          I agree that it's more conceptually consistent to put the distance-related metrics in HDFS-specific code. However, we would have to develop an optimized thread-local mechanism to do this, to avoid causing a performance regression in HDFS stream performance. Perhaps it would be better to simply move this to HDFS's existing per-stream ReadStatistics for now.

          Show
          cmccabe Colin P. McCabe added a comment - I filed HADOOP-13140 to track the effort. Thanks, Mingliang Liu . Ming Ma wrote: BTW, is the network distance metrics something general for all file system or it is more specific to HDFS? For example, local file system doesn't need that. If it is more HDFS specific, wonder if we should move it to HDFS specific metrics. I agree that it's more conceptually consistent to put the distance-related metrics in HDFS-specific code. However, we would have to develop an optimized thread-local mechanism to do this, to avoid causing a performance regression in HDFS stream performance. Perhaps it would be better to simply move this to HDFS's existing per-stream ReadStatistics for now.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks Ming Ma for the kind remind.

          IMHO the topology-aware read stats is HDFS only. You worked on the related jiras and would give us more convincing opinions. If it's true, we can create a new StorageStatistics, say RackAwareReadStorageStatistics, for the stats in HDFS-9579. We can track this effort in HADOOP-13031 after I modify the description soon. For the implementation, I'm not quite sure whether we can simply use the AtomicLongs instead of thread-local optimizations.

          Show
          liuml07 Mingliang Liu added a comment - Thanks Ming Ma for the kind remind. IMHO the topology-aware read stats is HDFS only. You worked on the related jiras and would give us more convincing opinions. If it's true, we can create a new StorageStatistics , say RackAwareReadStorageStatistics , for the stats in HDFS-9579 . We can track this effort in HADOOP-13031 after I modify the description soon. For the implementation, I'm not quite sure whether we can simply use the AtomicLongs instead of thread-local optimizations.
          Hide
          mingma Ming Ma added a comment -

          A minor note, given HDFS-9579 is only available in release 2.9 while this improvement is added to release 2.8, the network distance metrics for HDFS won't be useful until 2.9.

          BTW, is the network distance metrics something general for all file system or it is more specific to HDFS? For example, local file system doesn't need that. If it is more HDFS specific, wonder if we should move it to HDFS specific metrics.

          Show
          mingma Ming Ma added a comment - A minor note, given HDFS-9579 is only available in release 2.9 while this improvement is added to release 2.8, the network distance metrics for HDFS won't be useful until 2.9. BTW, is the network distance metrics something general for all file system or it is more specific to HDFS? For example, local file system doesn't need that. If it is more HDFS specific, wonder if we should move it to HDFS specific metrics.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I'm just hooking this up to S3A, with the actual data being retained in the S3AInstrumentation.

          One thing I'd like is to be confident that there were no retained instances when an FS gets deleted? Does that happen? or put differently, "how can I Get the storage statistics lifecycle to match that of the specific FS instance"

          Show
          stevel@apache.org Steve Loughran added a comment - I'm just hooking this up to S3A, with the actual data being retained in the S3AInstrumentation. One thing I'd like is to be confident that there were no retained instances when an FS gets deleted? Does that happen? or put differently, "how can I Get the storage statistics lifecycle to match that of the specific FS instance"
          Hide
          liuml07 Mingliang Liu added a comment -

          Thank you Brahma Reddy Battula for reporting this error. The Jenkins did not report this and I omitted that test.

          As Steve Loughran suggested, we need to guard the null scheme and throw a meaningful error if needed.

          I filed HADOOP-13140 to track the effort.

          Show
          liuml07 Mingliang Liu added a comment - Thank you Brahma Reddy Battula for reporting this error. The Jenkins did not report this and I omitted that test. As Steve Loughran suggested, we need to guard the null scheme and throw a meaningful error if needed. I filed HADOOP-13140 to track the effort.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          ouch. That means the key is null....which can happen if URI.getScheme() == null. which is happening in the test

          fs.initialize(new URI("/"), conf);
          

          Given the test can be fixed to have a valid URI, it's probably not enough of a breakage to merit a (big) rollback. But alongside the test the GlobalStorageStatistics code needs to set up to handle null schemes with at least a meaningful error

          Show
          stevel@apache.org Steve Loughran added a comment - ouch. That means the key is null....which can happen if URI.getScheme() == null . which is happening in the test fs.initialize( new URI( "/" ), conf); Given the test can be fixed to have a valid URI, it's probably not enough of a breakage to merit a (big) rollback. But alongside the test the GlobalStorageStatistics code needs to set up to handle null schemes with at least a meaningful error
          Hide
          brahmareddy Brahma Reddy Battula added a comment -

          AFAIK Following is started failing from this jira in...If you agree, will raise another jira to track this..scheme is getting null here...

          java.lang.NullPointerException: null
          	at java.util.TreeMap.getEntry(TreeMap.java:342)
          	at java.util.TreeMap.get(TreeMap.java:273)
          	at org.apache.hadoop.fs.GlobalStorageStatistics.put(GlobalStorageStatistics.java:73)
          	at org.apache.hadoop.fs.FileSystem.getStatistics(FileSystem.java:3598)
          	at org.apache.hadoop.fs.FileSystem.initialize(FileSystem.java:214)
          	at org.apache.hadoop.fs.RawLocalFileSystem.initialize(RawLocalFileSystem.java:101)
          	at org.apache.hadoop.yarn.server.applicationhistoryservice.TestFileSystemApplicationHistoryStore.initAndStartStore(TestFileSystemApplicationHistoryStore.java:70)
          	at org.apache.hadoop.yarn.server.applicationhistoryservice.TestFileSystemApplicationHistoryStore.setup(TestFileSystemApplicationHistoryStore.java:64)
          
          Show
          brahmareddy Brahma Reddy Battula added a comment - AFAIK Following is started failing from this jira in...If you agree, will raise another jira to track this..scheme is getting null here... java.lang.NullPointerException: null at java.util.TreeMap.getEntry(TreeMap.java:342) at java.util.TreeMap.get(TreeMap.java:273) at org.apache.hadoop.fs.GlobalStorageStatistics.put(GlobalStorageStatistics.java:73) at org.apache.hadoop.fs.FileSystem.getStatistics(FileSystem.java:3598) at org.apache.hadoop.fs.FileSystem.initialize(FileSystem.java:214) at org.apache.hadoop.fs.RawLocalFileSystem.initialize(RawLocalFileSystem.java:101) at org.apache.hadoop.yarn.server.applicationhistoryservice.TestFileSystemApplicationHistoryStore.initAndStartStore(TestFileSystemApplicationHistoryStore.java:70) at org.apache.hadoop.yarn.server.applicationhistoryservice.TestFileSystemApplicationHistoryStore.setup(TestFileSystemApplicationHistoryStore.java:64)
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9748 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9748/)
          HADOOP-13065. Add a new interface for retrieving FS and FC Statistics (cmccabe: rev 687233f20d24c29041929dd0a99d963cec54b6df)

          • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/StorageStatistics.java
          • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/EmptyStorageStatistics.java
          • hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystem.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/UnionStorageStatistics.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java
          • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSOpsCountStatistics.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystemStorageStatistics.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/GlobalStorageStatistics.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9748 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9748/ ) HADOOP-13065 . Add a new interface for retrieving FS and FC Statistics (cmccabe: rev 687233f20d24c29041929dd0a99d963cec54b6df) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/StorageStatistics.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/EmptyStorageStatistics.java hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDistributedFileSystem.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestHarFileSystem.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/UnionStorageStatistics.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystem.java hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSOpsCountStatistics.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileSystemStorageStatistics.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFilterFileSystem.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/GlobalStorageStatistics.java
          Hide
          liuml07 Mingliang Liu added a comment -

          Big TY Colin P. McCabe for your insightful discussion, contribution to the new stats design, and reviewing the patch. Actually I'd prefer a commit message like "contributed by Colin Patrick McCabe and Mingliang".

          Show
          liuml07 Mingliang Liu added a comment - Big TY Colin P. McCabe for your insightful discussion, contribution to the new stats design, and reviewing the patch. Actually I'd prefer a commit message like "contributed by Colin Patrick McCabe and Mingliang".
          Hide
          cmccabe Colin P. McCabe added a comment -

          committed to 2.8. Thanks, Mingliang Liu.

          Show
          cmccabe Colin P. McCabe added a comment - committed to 2.8. Thanks, Mingliang Liu .
          Hide
          cmccabe Colin P. McCabe added a comment -

          +1 for version 13. Thanks, Mingliang Liu.

          Show
          cmccabe Colin P. McCabe added a comment - +1 for version 13. Thanks, Mingliang Liu .
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 10s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 1m 25s Maven dependency ordering for branch
          +1 mvninstall 7m 12s trunk passed
          +1 compile 6m 46s trunk passed with JDK v1.8.0_91
          +1 compile 7m 26s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 39s trunk passed
          +1 mvnsite 2m 35s trunk passed
          +1 mvneclipse 0m 41s trunk passed
          +1 findbugs 5m 15s trunk passed
          +1 javadoc 2m 24s trunk passed with JDK v1.8.0_91
          +1 javadoc 3m 16s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 1m 59s the patch passed
          +1 compile 6m 17s the patch passed with JDK v1.8.0_91
          -1 javac 6m 17s root-jdk1.8.0_91 with JDK v1.8.0_91 generated 8 new + 663 unchanged - 0 fixed = 671 total (was 663)
          +1 compile 6m 50s the patch passed with JDK v1.7.0_95
          -1 javac 6m 50s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 8 new + 672 unchanged - 0 fixed = 680 total (was 672)
          -1 checkstyle 1m 28s root: The patch generated 3 new + 435 unchanged - 2 fixed = 438 total (was 437)
          +1 mvnsite 2m 21s the patch passed
          +1 mvneclipse 0m 40s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 5m 58s the patch passed
          +1 javadoc 2m 32s the patch passed with JDK v1.8.0_91
          +1 javadoc 3m 21s the patch passed with JDK v1.7.0_95
          +1 unit 9m 1s hadoop-common in the patch passed with JDK v1.8.0_91.
          +1 unit 1m 9s hadoop-hdfs-client in the patch passed with JDK v1.8.0_91.
          -1 unit 64m 10s hadoop-hdfs in the patch failed with JDK v1.8.0_91.
          -1 unit 7m 45s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 unit 1m 1s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 55m 57s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 26s The patch does not generate ASF License warnings.
          211m 37s



          Reason Tests
          JDK v1.8.0_91 Failed junit tests hadoop.hdfs.server.namenode.TestEditLog
            hadoop.hdfs.server.datanode.TestDataNodeUUID
            hadoop.hdfs.server.balancer.TestBalancer
          JDK v1.7.0_95 Failed junit tests hadoop.net.TestDNS
            hadoop.hdfs.server.balancer.TestBalancer
            hadoop.metrics2.sink.TestRollingFileSystemSinkWithSecureHdfs



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803315/HADOOP-13065.013.patch
          JIRA Issue HADOOP-13065
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux dd25552accec 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 / 6b1c1cb
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_91.txt
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/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/9361/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/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/9361/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/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 10s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 1m 25s Maven dependency ordering for branch +1 mvninstall 7m 12s trunk passed +1 compile 6m 46s trunk passed with JDK v1.8.0_91 +1 compile 7m 26s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 39s trunk passed +1 mvnsite 2m 35s trunk passed +1 mvneclipse 0m 41s trunk passed +1 findbugs 5m 15s trunk passed +1 javadoc 2m 24s trunk passed with JDK v1.8.0_91 +1 javadoc 3m 16s trunk passed with JDK v1.7.0_95 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 59s the patch passed +1 compile 6m 17s the patch passed with JDK v1.8.0_91 -1 javac 6m 17s root-jdk1.8.0_91 with JDK v1.8.0_91 generated 8 new + 663 unchanged - 0 fixed = 671 total (was 663) +1 compile 6m 50s the patch passed with JDK v1.7.0_95 -1 javac 6m 50s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 8 new + 672 unchanged - 0 fixed = 680 total (was 672) -1 checkstyle 1m 28s root: The patch generated 3 new + 435 unchanged - 2 fixed = 438 total (was 437) +1 mvnsite 2m 21s the patch passed +1 mvneclipse 0m 40s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 5m 58s the patch passed +1 javadoc 2m 32s the patch passed with JDK v1.8.0_91 +1 javadoc 3m 21s the patch passed with JDK v1.7.0_95 +1 unit 9m 1s hadoop-common in the patch passed with JDK v1.8.0_91. +1 unit 1m 9s hadoop-hdfs-client in the patch passed with JDK v1.8.0_91. -1 unit 64m 10s hadoop-hdfs in the patch failed with JDK v1.8.0_91. -1 unit 7m 45s hadoop-common in the patch failed with JDK v1.7.0_95. +1 unit 1m 1s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 55m 57s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 26s The patch does not generate ASF License warnings. 211m 37s Reason Tests JDK v1.8.0_91 Failed junit tests hadoop.hdfs.server.namenode.TestEditLog   hadoop.hdfs.server.datanode.TestDataNodeUUID   hadoop.hdfs.server.balancer.TestBalancer JDK v1.7.0_95 Failed junit tests hadoop.net.TestDNS   hadoop.hdfs.server.balancer.TestBalancer   hadoop.metrics2.sink.TestRollingFileSystemSinkWithSecureHdfs Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12803315/HADOOP-13065.013.patch JIRA Issue HADOOP-13065 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux dd25552accec 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 / 6b1c1cb Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_91.txt javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/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/9361/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/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/9361/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9361/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks Colin P. McCabe for the review.

          The v13 patch is to address the null return value problem. Yes we should return null in this case, indicating the operation is not tracked. Zero-value is for the case where the operation is tracked by the counter is not yet incremented. This is a good catch.

          For this use case request, the per-operation stats is per job and thus is shared among different filesystem instances. Based on the current stats design, we can further support per-instance stats as follow-on jiras if new use cases appear.

          Show
          liuml07 Mingliang Liu added a comment - Thanks Colin P. McCabe for the review. The v13 patch is to address the null return value problem. Yes we should return null in this case, indicating the operation is not tracked. Zero-value is for the case where the operation is tracked by the counter is not yet incremented. This is a good catch. For this use case request, the per-operation stats is per job and thus is shared among different filesystem instances. Based on the current stats design, we can further support per-instance stats as follow-on jiras if new use cases appear.
          Hide
          cmccabe Colin P. McCabe added a comment - - edited

          Thanks, Mingliang Liu. DFSOpsCountStatistics is a nice implementation. It's also nice to have this for webhdfs as well.

          156  @Override
          157	  public Long getLong(String key) {
          158	    final OpType type = OpType.fromSymbol(key);
          159	    return type == null ? 0L : opsCount.get(type).get();
          160	  }
          

          I think this should return null in the case where type == null, right? Indicating that there is no such statistic.

          159	    storageStatistics = (DFSOpsCountStatistics) GlobalStorageStatistics.INSTANCE
          160	        .put(DFSOpsCountStatistics.NAME,
          161	          new StorageStatisticsProvider() {
          162	            @Override
          163	            public StorageStatistics provide() {
          164	              return new DFSOpsCountStatistics();
          165	            }
          166	          });
          

          Hmm, I wonder if these StorageStatistics objects should be per-FS-instance rather than per-class? I guess let's do that in a follow-on, though, after this gets committed.

          +1 for HADOOP-13065.012.patch once the null thing is fixed

          Show
          cmccabe Colin P. McCabe added a comment - - edited Thanks, Mingliang Liu . DFSOpsCountStatistics is a nice implementation. It's also nice to have this for webhdfs as well. 156 @Override 157 public Long getLong( String key) { 158 final OpType type = OpType.fromSymbol(key); 159 return type == null ? 0L : opsCount.get(type).get(); 160 } I think this should return null in the case where type == null, right? Indicating that there is no such statistic. 159 storageStatistics = (DFSOpsCountStatistics) GlobalStorageStatistics.INSTANCE 160 .put(DFSOpsCountStatistics.NAME, 161 new StorageStatisticsProvider() { 162 @Override 163 public StorageStatistics provide() { 164 return new DFSOpsCountStatistics(); 165 } 166 }); Hmm, I wonder if these StorageStatistics objects should be per-FS-instance rather than per-class? I guess let's do that in a follow-on, though, after this gets committed. +1 for HADOOP-13065 .012.patch once the null thing is fixed
          Hide
          liuml07 Mingliang Liu added a comment -

          The testing failures seem not related. Jitendra Nath Pandey and Colin P. McCabe, can you kindly comment the current patch? I'd be happy work on the follow-up jiras after this one is committed. Thanks.

          Show
          liuml07 Mingliang Liu added a comment - The testing failures seem not related. Jitendra Nath Pandey and Colin P. McCabe , can you kindly comment the current patch? I'd be happy work on the follow-up jiras after this one is committed. Thanks.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 52s Maven dependency ordering for branch
          +1 mvninstall 8m 26s trunk passed
          +1 compile 9m 19s trunk passed with JDK v1.8.0_91
          +1 compile 8m 48s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 49s trunk passed
          +1 mvnsite 3m 3s trunk passed
          +1 mvneclipse 0m 48s trunk passed
          +1 findbugs 6m 20s trunk passed
          +1 javadoc 3m 7s trunk passed with JDK v1.8.0_91
          +1 javadoc 4m 13s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 2m 31s the patch passed
          +1 compile 9m 9s the patch passed with JDK v1.8.0_91
          -1 javac 9m 9s root-jdk1.8.0_91 with JDK v1.8.0_91 generated 8 new + 662 unchanged - 0 fixed = 670 total (was 662)
          +1 compile 8m 49s the patch passed with JDK v1.7.0_95
          -1 javac 8m 49s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 8 new + 672 unchanged - 0 fixed = 680 total (was 672)
          -1 checkstyle 1m 48s root: The patch generated 3 new + 435 unchanged - 2 fixed = 438 total (was 437)
          +1 mvnsite 2m 59s the patch passed
          +1 mvneclipse 0m 48s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 7m 10s the patch passed
          +1 javadoc 3m 1s the patch passed with JDK v1.8.0_91
          +1 javadoc 4m 7s the patch passed with JDK v1.7.0_95
          +1 unit 10m 29s hadoop-common in the patch passed with JDK v1.8.0_91.
          +1 unit 1m 11s hadoop-hdfs-client in the patch passed with JDK v1.8.0_91.
          -1 unit 77m 51s hadoop-hdfs in the patch failed with JDK v1.8.0_91.
          +1 unit 10m 4s hadoop-common in the patch passed with JDK v1.7.0_95.
          +1 unit 1m 12s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 77m 38s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 34s The patch does not generate ASF License warnings.
          268m 33s



          Reason Tests
          JDK v1.8.0_91 Failed junit tests hadoop.hdfs.TestFileAppend
            hadoop.hdfs.TestDFSUpgradeFromImage
            hadoop.hdfs.TestAsyncDFSRename
            hadoop.hdfs.server.namenode.TestEditLog
            hadoop.hdfs.server.datanode.TestDataNodeLifeline
            hadoop.hdfs.server.datanode.TestDirectoryScanner
          JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency
            hadoop.hdfs.TestAsyncDFSRename
            hadoop.hdfs.server.namenode.TestEditLog
            hadoop.hdfs.server.datanode.TestDirectoryScanner
            hadoop.hdfs.security.TestDelegationTokenForProxyUser



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12802784/HADOOP-13065.012.patch
          JIRA Issue HADOOP-13065
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux de715d737572 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 / 6957e45
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_91.txt
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/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/9320/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/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/9320/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/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 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 52s Maven dependency ordering for branch +1 mvninstall 8m 26s trunk passed +1 compile 9m 19s trunk passed with JDK v1.8.0_91 +1 compile 8m 48s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 49s trunk passed +1 mvnsite 3m 3s trunk passed +1 mvneclipse 0m 48s trunk passed +1 findbugs 6m 20s trunk passed +1 javadoc 3m 7s trunk passed with JDK v1.8.0_91 +1 javadoc 4m 13s trunk passed with JDK v1.7.0_95 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 2m 31s the patch passed +1 compile 9m 9s the patch passed with JDK v1.8.0_91 -1 javac 9m 9s root-jdk1.8.0_91 with JDK v1.8.0_91 generated 8 new + 662 unchanged - 0 fixed = 670 total (was 662) +1 compile 8m 49s the patch passed with JDK v1.7.0_95 -1 javac 8m 49s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 8 new + 672 unchanged - 0 fixed = 680 total (was 672) -1 checkstyle 1m 48s root: The patch generated 3 new + 435 unchanged - 2 fixed = 438 total (was 437) +1 mvnsite 2m 59s the patch passed +1 mvneclipse 0m 48s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 7m 10s the patch passed +1 javadoc 3m 1s the patch passed with JDK v1.8.0_91 +1 javadoc 4m 7s the patch passed with JDK v1.7.0_95 +1 unit 10m 29s hadoop-common in the patch passed with JDK v1.8.0_91. +1 unit 1m 11s hadoop-hdfs-client in the patch passed with JDK v1.8.0_91. -1 unit 77m 51s hadoop-hdfs in the patch failed with JDK v1.8.0_91. +1 unit 10m 4s hadoop-common in the patch passed with JDK v1.7.0_95. +1 unit 1m 12s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 77m 38s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 34s The patch does not generate ASF License warnings. 268m 33s Reason Tests JDK v1.8.0_91 Failed junit tests hadoop.hdfs.TestFileAppend   hadoop.hdfs.TestDFSUpgradeFromImage   hadoop.hdfs.TestAsyncDFSRename   hadoop.hdfs.server.namenode.TestEditLog   hadoop.hdfs.server.datanode.TestDataNodeLifeline   hadoop.hdfs.server.datanode.TestDirectoryScanner JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.namenode.TestNameNodeMetadataConsistency   hadoop.hdfs.TestAsyncDFSRename   hadoop.hdfs.server.namenode.TestEditLog   hadoop.hdfs.server.datanode.TestDirectoryScanner   hadoop.hdfs.security.TestDelegationTokenForProxyUser Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12802784/HADOOP-13065.012.patch JIRA Issue HADOOP-13065 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux de715d737572 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 / 6957e45 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_91.txt javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/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/9320/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/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/9320/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9320/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Test failures are not related. The javac warnings are caused by deprecating getStatistics() API, and thus are expected. The v12 patch removes useless imports.

          Show
          liuml07 Mingliang Liu added a comment - Test failures are not related. The javac warnings are caused by deprecating getStatistics() API, and thus are expected. The v12 patch removes useless imports.
          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 3 new or modified test files.
          0 mvndep 0m 17s Maven dependency ordering for branch
          +1 mvninstall 7m 1s trunk passed
          +1 compile 5m 54s trunk passed with JDK v1.8.0_91
          +1 compile 7m 1s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 7s trunk passed
          +1 mvnsite 2m 27s trunk passed
          +1 mvneclipse 0m 41s trunk passed
          +1 findbugs 5m 12s trunk passed
          +1 javadoc 2m 25s trunk passed with JDK v1.8.0_91
          +1 javadoc 3m 14s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 1m 58s the patch passed
          +1 compile 5m 58s the patch passed with JDK v1.8.0_91
          -1 javac 5m 58s root-jdk1.8.0_91 with JDK v1.8.0_91 generated 8 new + 662 unchanged - 0 fixed = 670 total (was 662)
          +1 compile 6m 46s the patch passed with JDK v1.7.0_95
          -1 javac 6m 46s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 8 new + 672 unchanged - 0 fixed = 680 total (was 672)
          -1 checkstyle 1m 8s root: The patch generated 2 new + 206 unchanged - 2 fixed = 208 total (was 208)
          +1 mvnsite 2m 21s the patch passed
          +1 mvneclipse 0m 41s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 5m 52s the patch passed
          +1 javadoc 2m 25s the patch passed with JDK v1.8.0_91
          +1 javadoc 3m 23s the patch passed with JDK v1.7.0_95
          +1 unit 7m 49s hadoop-common in the patch passed with JDK v1.8.0_91.
          +1 unit 0m 53s hadoop-hdfs-client in the patch passed with JDK v1.8.0_91.
          -1 unit 58m 17s hadoop-hdfs in the patch failed with JDK v1.8.0_91.
          +1 unit 7m 45s hadoop-common in the patch passed with JDK v1.7.0_95.
          +1 unit 0m 59s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 55m 28s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 26s The patch does not generate ASF License warnings.
          199m 33s



          Reason Tests
          JDK v1.8.0_91 Failed junit tests hadoop.hdfs.shortcircuit.TestShortCircuitCache
            hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaRecovery
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl
          JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12802738/HADOOP-13065.011.patch
          JIRA Issue HADOOP-13065
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux d4e0f53b0cfe 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 / 1c5bbf6
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_91.txt
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/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/9314/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/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/9314/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/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 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 3 new or modified test files. 0 mvndep 0m 17s Maven dependency ordering for branch +1 mvninstall 7m 1s trunk passed +1 compile 5m 54s trunk passed with JDK v1.8.0_91 +1 compile 7m 1s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 7s trunk passed +1 mvnsite 2m 27s trunk passed +1 mvneclipse 0m 41s trunk passed +1 findbugs 5m 12s trunk passed +1 javadoc 2m 25s trunk passed with JDK v1.8.0_91 +1 javadoc 3m 14s trunk passed with JDK v1.7.0_95 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 58s the patch passed +1 compile 5m 58s the patch passed with JDK v1.8.0_91 -1 javac 5m 58s root-jdk1.8.0_91 with JDK v1.8.0_91 generated 8 new + 662 unchanged - 0 fixed = 670 total (was 662) +1 compile 6m 46s the patch passed with JDK v1.7.0_95 -1 javac 6m 46s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 8 new + 672 unchanged - 0 fixed = 680 total (was 672) -1 checkstyle 1m 8s root: The patch generated 2 new + 206 unchanged - 2 fixed = 208 total (was 208) +1 mvnsite 2m 21s the patch passed +1 mvneclipse 0m 41s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 5m 52s the patch passed +1 javadoc 2m 25s the patch passed with JDK v1.8.0_91 +1 javadoc 3m 23s the patch passed with JDK v1.7.0_95 +1 unit 7m 49s hadoop-common in the patch passed with JDK v1.8.0_91. +1 unit 0m 53s hadoop-hdfs-client in the patch passed with JDK v1.8.0_91. -1 unit 58m 17s hadoop-hdfs in the patch failed with JDK v1.8.0_91. +1 unit 7m 45s hadoop-common in the patch passed with JDK v1.7.0_95. +1 unit 0m 59s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 55m 28s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 26s The patch does not generate ASF License warnings. 199m 33s Reason Tests JDK v1.8.0_91 Failed junit tests hadoop.hdfs.shortcircuit.TestShortCircuitCache   hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaRecovery   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl JDK v1.7.0_95 Failed junit tests hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12802738/HADOOP-13065.011.patch JIRA Issue HADOOP-13065 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux d4e0f53b0cfe 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 / 1c5bbf6 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_91.txt javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/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/9314/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/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/9314/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9314/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks Colin P. McCabe for the suggestion.

          ...the old API would become deprecated in branch-2, and removed in branch-3.

          That makes perfect sense.

          There isn't any need to change the annotation since we don't plan to modify the interface, just remove it.

          The v11 patch removed the Stable annotation for the GlobalStorageStatistics and StorageStatistics.

          ...so that they could be used in both FileContext and FileSystem.

          This is a valid point that I missed before. Will track this in the HADOOP-13031.

          Show
          liuml07 Mingliang Liu added a comment - Thanks Colin P. McCabe for the suggestion. ...the old API would become deprecated in branch-2, and removed in branch-3. That makes perfect sense. There isn't any need to change the annotation since we don't plan to modify the interface, just remove it. The v11 patch removed the Stable annotation for the GlobalStorageStatistics and StorageStatistics . ...so that they could be used in both FileContext and FileSystem. This is a valid point that I missed before. Will track this in the HADOOP-13031 .
          Hide
          cmccabe Colin P. McCabe added a comment -

          One quick question is that, some of the storage statistics classes (e.g. GlobalStorageStatistics are annotated as Stable, do we have to be a bit more conservative by making them Unstable before ultimately removing the Statistics?

          Good question. I think that what would happen is that the old API would become deprecated in branch-2, and removed in branch-3. There isn't any need to change the annotation since we don't plan to modify the interface, just remove it.

          As follow-on work, 1. We can move the rack-awareness read bytes to a separate storage statistics as it's only used by HDFS, and 2. We can remove Statistics API, but keep the thread local implementation in FileSystemStorageStatistics class.

          That makes sense. One thing that we've talked about doing in the past is moving these statistics to a separate java file, so that they could be used in both FileContext and FileSystem. Maybe we could call them something like ThreadLocalFsStatistics or something?

          Show
          cmccabe Colin P. McCabe added a comment - One quick question is that, some of the storage statistics classes (e.g. GlobalStorageStatistics are annotated as Stable, do we have to be a bit more conservative by making them Unstable before ultimately removing the Statistics? Good question. I think that what would happen is that the old API would become deprecated in branch-2, and removed in branch-3. There isn't any need to change the annotation since we don't plan to modify the interface, just remove it. As follow-on work, 1. We can move the rack-awareness read bytes to a separate storage statistics as it's only used by HDFS, and 2. We can remove Statistics API, but keep the thread local implementation in FileSystemStorageStatistics class. That makes sense. One thing that we've talked about doing in the past is moving these statistics to a separate java file, so that they could be used in both FileContext and FileSystem. Maybe we could call them something like ThreadLocalFsStatistics or something?
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks Colin P. McCabe for the comment. The v10 patch deprecates the getStatistics() API, and fixes the simple checkstyle warning.

          One quick question is that, some of the storage statistics classes (e.g. GlobalStorageStatistics are annotated as Stable, do we have to be a bit more conservative by making them Unstable before ultimately removing the Statistics?

          As follow-on work,

          1. We can move the rack-awareness read bytes to a separate storage statistics as it's only used by HDFS
          2. We can remove Statistics API, but keep the thread local implementation in FileSystemStorageStatistics class.

          I will update the previously filed jiras HADOOP-13032 and HADOOP-13031 accordingly after this patch is in the trunk.

          Show
          liuml07 Mingliang Liu added a comment - Thanks Colin P. McCabe for the comment. The v10 patch deprecates the getStatistics() API, and fixes the simple checkstyle warning. One quick question is that, some of the storage statistics classes (e.g. GlobalStorageStatistics are annotated as Stable , do we have to be a bit more conservative by making them Unstable before ultimately removing the Statistics? As follow-on work, We can move the rack-awareness read bytes to a separate storage statistics as it's only used by HDFS We can remove Statistics API, but keep the thread local implementation in FileSystemStorageStatistics class. I will update the previously filed jiras HADOOP-13032 and HADOOP-13031 accordingly after this patch is in the trunk.
          Hide
          cmccabe Colin P. McCabe added a comment -

          Thanks for the reviews.

          in FileSystem.getStatistics(), For performance, you could try using ConcurrentMap for the map, and only if it is not present create the objects and call putIfAbsent() (or a synchronized block create and update the maps (with a second lookup there to eliminate the small race condition). This will eliminate the sync point on a simple lookup when the entry exists.

          Hmm. I don't think that we really need to optimize this function. When using the new API, the only time this function gets called is when a new FileSystem object is created, which should be very rare.

          For testing a may to reset/remove an entry could be handy.

          We do have some tests that zero out the existing statistics objects. I'm not sure if removing the entry really gets us more coverage than we have now, since we know that it was created by this code path (therefore the code path was tested).

          That's said, we can firstly deprecate the FileSystem#getStatistics()?

          Agree.

          Show
          cmccabe Colin P. McCabe added a comment - Thanks for the reviews. in FileSystem.getStatistics(), For performance, you could try using ConcurrentMap for the map, and only if it is not present create the objects and call putIfAbsent() (or a synchronized block create and update the maps (with a second lookup there to eliminate the small race condition). This will eliminate the sync point on a simple lookup when the entry exists. Hmm. I don't think that we really need to optimize this function. When using the new API, the only time this function gets called is when a new FileSystem object is created, which should be very rare. For testing a may to reset/remove an entry could be handy. We do have some tests that zero out the existing statistics objects. I'm not sure if removing the entry really gets us more coverage than we have now, since we know that it was created by this code path (therefore the code path was tested). That's said, we can firstly deprecate the FileSystem#getStatistics()? Agree.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 12s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 25s Maven dependency ordering for branch
          +1 mvninstall 6m 30s trunk passed
          +1 compile 5m 39s trunk passed with JDK v1.8.0_91
          +1 compile 6m 36s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 6s trunk passed
          +1 mvnsite 2m 24s trunk passed
          +1 mvneclipse 0m 41s trunk passed
          +1 findbugs 5m 6s trunk passed
          +1 javadoc 2m 18s trunk passed with JDK v1.8.0_91
          +1 javadoc 3m 12s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 1m 56s the patch passed
          +1 compile 5m 54s the patch passed with JDK v1.8.0_91
          +1 javac 5m 54s the patch passed
          +1 compile 6m 52s the patch passed with JDK v1.7.0_95
          +1 javac 6m 52s the patch passed
          -1 checkstyle 1m 8s root: The patch generated 1 new + 207 unchanged - 1 fixed = 208 total (was 208)
          +1 mvnsite 2m 22s the patch passed
          +1 mvneclipse 0m 40s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 5m 49s the patch passed
          +1 javadoc 2m 20s the patch passed with JDK v1.8.0_91
          +1 javadoc 3m 14s the patch passed with JDK v1.7.0_95
          +1 unit 7m 50s hadoop-common in the patch passed with JDK v1.8.0_91.
          +1 unit 0m 51s hadoop-hdfs-client in the patch passed with JDK v1.8.0_91.
          -1 unit 57m 9s hadoop-hdfs in the patch failed with JDK v1.8.0_91.
          -1 unit 7m 40s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 unit 1m 1s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 54m 36s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          -1 asflicense 0m 26s The patch generated 1 ASF License warnings.
          195m 49s



          Reason Tests
          JDK v1.8.0_91 Failed junit tests hadoop.hdfs.web.TestWebHDFS
            hadoop.hdfs.TestHFlush
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl
            hadoop.hdfs.TestDFSUpgradeFromImage
          JDK v1.7.0_95 Failed junit tests hadoop.net.TestDNS
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12802355/HADOOP-13065.009.patch
          JIRA Issue HADOOP-13065
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 8b7e00deed59 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 / 1268cf5
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/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/9285/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/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/9285/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/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 12s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 25s Maven dependency ordering for branch +1 mvninstall 6m 30s trunk passed +1 compile 5m 39s trunk passed with JDK v1.8.0_91 +1 compile 6m 36s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 6s trunk passed +1 mvnsite 2m 24s trunk passed +1 mvneclipse 0m 41s trunk passed +1 findbugs 5m 6s trunk passed +1 javadoc 2m 18s trunk passed with JDK v1.8.0_91 +1 javadoc 3m 12s trunk passed with JDK v1.7.0_95 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 56s the patch passed +1 compile 5m 54s the patch passed with JDK v1.8.0_91 +1 javac 5m 54s the patch passed +1 compile 6m 52s the patch passed with JDK v1.7.0_95 +1 javac 6m 52s the patch passed -1 checkstyle 1m 8s root: The patch generated 1 new + 207 unchanged - 1 fixed = 208 total (was 208) +1 mvnsite 2m 22s the patch passed +1 mvneclipse 0m 40s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 5m 49s the patch passed +1 javadoc 2m 20s the patch passed with JDK v1.8.0_91 +1 javadoc 3m 14s the patch passed with JDK v1.7.0_95 +1 unit 7m 50s hadoop-common in the patch passed with JDK v1.8.0_91. +1 unit 0m 51s hadoop-hdfs-client in the patch passed with JDK v1.8.0_91. -1 unit 57m 9s hadoop-hdfs in the patch failed with JDK v1.8.0_91. -1 unit 7m 40s hadoop-common in the patch failed with JDK v1.7.0_95. +1 unit 1m 1s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 54m 36s hadoop-hdfs in the patch failed with JDK v1.7.0_95. -1 asflicense 0m 26s The patch generated 1 ASF License warnings. 195m 49s Reason Tests JDK v1.8.0_91 Failed junit tests hadoop.hdfs.web.TestWebHDFS   hadoop.hdfs.TestHFlush   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl   hadoop.hdfs.TestDFSUpgradeFromImage JDK v1.7.0_95 Failed junit tests hadoop.net.TestDNS   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12802355/HADOOP-13065.009.patch JIRA Issue HADOOP-13065 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 8b7e00deed59 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 / 1268cf5 Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/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/9285/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/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/9285/testReport/ asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9285/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thankw Steve Loughran very much for your comment. The v9 patch addresses your 2nd comment on test (which is a very nice catch), checkstyle and findbugs warnings, along with simple fix for the related test failures. Again, thanks Colin P. McCabe for the new design.

          As to your concurrent map comment, I shall update the patch later if I get your point. By the map, I suppose we're talking about the GlobalStorageStatistics#map. My concern is that, the FileSystem.getStatistics() itself is static and synchronized, and the GlobalStorageStatistics#map will be looked up and updated iff there is no entry in the FileSystem#statisticsTable. So basically if an entry exists, there should be a respective entry in FileSystem#statisticsTable, and thus no look up is issued. Ideally as Colin P. McCabe suggested, we should remove the FileSystem#Statistics as a public interface. For that I think we will refactor this part of code heavily.
          That's said, we can firstly deprecate the FileSystem#getStatistics()?

          Show
          liuml07 Mingliang Liu added a comment - Thankw Steve Loughran very much for your comment. The v9 patch addresses your 2nd comment on test (which is a very nice catch), checkstyle and findbugs warnings, along with simple fix for the related test failures. Again, thanks Colin P. McCabe for the new design. As to your concurrent map comment, I shall update the patch later if I get your point. By the map, I suppose we're talking about the GlobalStorageStatistics#map . My concern is that, the FileSystem.getStatistics() itself is static and synchronized , and the GlobalStorageStatistics#map will be looked up and updated iff there is no entry in the FileSystem#statisticsTable . So basically if an entry exists, there should be a respective entry in FileSystem#statisticsTable , and thus no look up is issued. Ideally as Colin P. McCabe suggested, we should remove the FileSystem#Statistics as a public interface. For that I think we will refactor this part of code heavily. That's said, we can firstly deprecate the FileSystem#getStatistics() ?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I like this patch, especially the isTracked() probe.

          in FileSystem.getStatistics()

          1. For performance, you could try using ConcurrentMap for the map, and only if it is not present create the objects and call putIfAbsent() (or a synchronized block create and update the maps (with a second lookup there to eliminate the small race condition). This will eliminate the sync point on a simple lookup when the entry exists.
          2. For testing a may to reset/remove an entry could be handy.

          In testConcurrentStatistics()

          in the runnables, line 737, there's a fail("Child failed with exception: " + t)

          1. tests shouldn't lose the inner stack. Just let it pass through
          2. and, as it will fail in a separate thread, isn't going to fail the test anyway, as far as I can tell

          Better to catch, store in a list of exceptions caught, and, once the allDone.await() checkpoint is reached, look at that list, if non-empty log all exceptions then throw the first one. That will promote it to a failure on the test thread.

          Show
          stevel@apache.org Steve Loughran added a comment - I like this patch, especially the isTracked() probe. in FileSystem.getStatistics() For performance, you could try using ConcurrentMap for the map, and only if it is not present create the objects and call putIfAbsent() (or a synchronized block create and update the maps (with a second lookup there to eliminate the small race condition). This will eliminate the sync point on a simple lookup when the entry exists. For testing a may to reset/remove an entry could be handy. In testConcurrentStatistics() in the runnables, line 737, there's a fail("Child failed with exception: " + t) tests shouldn't lose the inner stack. Just let it pass through and, as it will fail in a separate thread, isn't going to fail the test anyway, as far as I can tell Better to catch, store in a list of exceptions caught, and, once the allDone.await() checkpoint is reached, look at that list, if non-empty log all exceptions then throw the first one. That will promote it to a failure on the test thread.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          ALong has contention under load; it doesn't compile down to a simple x86 LOCK XADD $address, value, which is all you need. The updater stuff is designed to get closer to this with something eventually generated code like

          address = &object + offset-to-field
          LOCK XADD $address, value
          

          (no, I can't code x86 ASM. I just spent lots of time staring at it trying to debug windows C++ code)

          The Java 9 stuff is actually intended to something really profound: provide the same operations against arrays, so address = array + (aligned) offset, C's *(array+offset) ++ were the type of array some atomic long[].

          The fencing stuff is just there to make people who think they understand memory models, CPU and compiler re-ordering & the like write fast code. I've only ever seen anyone argue for doing that in user level code once Twitter: eventually consistent data structures and their code only worked because they didn't understand that in Java 5+, volatile reads are non-reorderable fences on all accesses. (that is: he gets the explanation of why things work wrong). Nobody should be going near that in the Hadoop code at all.

          Show
          stevel@apache.org Steve Loughran added a comment - ALong has contention under load; it doesn't compile down to a simple x86 LOCK XADD $address, value , which is all you need. The updater stuff is designed to get closer to this with something eventually generated code like address = &object + offset-to-field LOCK XADD $address, value (no, I can't code x86 ASM. I just spent lots of time staring at it trying to debug windows C++ code) The Java 9 stuff is actually intended to something really profound: provide the same operations against arrays, so address = array + (aligned) offset , C's *(array+offset) ++ were the type of array some atomic long[]. The fencing stuff is just there to make people who think they understand memory models, CPU and compiler re-ordering & the like write fast code. I've only ever seen anyone argue for doing that in user level code once Twitter: eventually consistent data structures and their code only worked because they didn't understand that in Java 5+, volatile reads are non-reorderable fences on all accesses. (that is: he gets the explanation of why things work wrong). Nobody should be going near that in the Hadoop code at all.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 8m 10s 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.
          0 mvndep 1m 33s Maven dependency ordering for branch
          +1 mvninstall 6m 59s trunk passed
          +1 compile 6m 28s trunk passed with JDK v1.8.0_91
          +1 compile 6m 48s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 6s trunk passed
          +1 mvnsite 2m 24s trunk passed
          +1 mvneclipse 0m 45s trunk passed
          +1 findbugs 5m 5s trunk passed
          +1 javadoc 2m 14s trunk passed with JDK v1.8.0_91
          +1 javadoc 3m 15s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 25s Maven dependency ordering for patch
          +1 mvninstall 1m 59s the patch passed
          +1 compile 6m 7s the patch passed with JDK v1.8.0_91
          +1 javac 6m 7s the patch passed
          +1 compile 6m 46s the patch passed with JDK v1.7.0_95
          +1 javac 6m 46s the patch passed
          -1 checkstyle 1m 9s root: The patch generated 33 new + 208 unchanged - 1 fixed = 241 total (was 209)
          +1 mvnsite 2m 18s the patch passed
          +1 mvneclipse 0m 39s the patch passed
          -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix.
          -1 findbugs 1m 51s hadoop-common-project/hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 2m 18s the patch passed with JDK v1.8.0_91
          +1 javadoc 3m 7s the patch passed with JDK v1.7.0_95
          -1 unit 6m 26s hadoop-common in the patch failed with JDK v1.8.0_91.
          +1 unit 0m 47s hadoop-hdfs-client in the patch passed with JDK v1.8.0_91.
          -1 unit 58m 18s hadoop-hdfs in the patch failed with JDK v1.8.0_91.
          -1 unit 6m 54s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 unit 0m 58s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 56m 59s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          -1 asflicense 0m 28s The patch generated 1 ASF License warnings.
          207m 51s



          Reason Tests
          FindBugs module:hadoop-common-project/hadoop-common
            Should org.apache.hadoop.fs.FileSystemStorageStatistics$LongStatisticIterator be a static inner class? At FileSystemStorageStatistics.java:inner class? At FileSystemStorageStatistics.java:[lines 51-78]
          JDK v1.8.0_91 Failed junit tests hadoop.fs.TestFilterFileSystem
            hadoop.fs.TestHarFileSystem
            hadoop.hdfs.TestFileAppend
            hadoop.hdfs.TestHFlush
          JDK v1.7.0_95 Failed junit tests hadoop.fs.TestFilterFileSystem
            hadoop.fs.TestHarFileSystem
            hadoop.hdfs.TestHFlush
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12802071/HADOOP-13065.008.patch
          JIRA Issue HADOOP-13065
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 4b827a907233 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 / ed54f5f
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/diff-checkstyle-root.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/whitespace-eol.txt
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/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/9268/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/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/9268/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/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 8m 10s 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. 0 mvndep 1m 33s Maven dependency ordering for branch +1 mvninstall 6m 59s trunk passed +1 compile 6m 28s trunk passed with JDK v1.8.0_91 +1 compile 6m 48s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 6s trunk passed +1 mvnsite 2m 24s trunk passed +1 mvneclipse 0m 45s trunk passed +1 findbugs 5m 5s trunk passed +1 javadoc 2m 14s trunk passed with JDK v1.8.0_91 +1 javadoc 3m 15s trunk passed with JDK v1.7.0_95 0 mvndep 0m 25s Maven dependency ordering for patch +1 mvninstall 1m 59s the patch passed +1 compile 6m 7s the patch passed with JDK v1.8.0_91 +1 javac 6m 7s the patch passed +1 compile 6m 46s the patch passed with JDK v1.7.0_95 +1 javac 6m 46s the patch passed -1 checkstyle 1m 9s root: The patch generated 33 new + 208 unchanged - 1 fixed = 241 total (was 209) +1 mvnsite 2m 18s the patch passed +1 mvneclipse 0m 39s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix. -1 findbugs 1m 51s hadoop-common-project/hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 2m 18s the patch passed with JDK v1.8.0_91 +1 javadoc 3m 7s the patch passed with JDK v1.7.0_95 -1 unit 6m 26s hadoop-common in the patch failed with JDK v1.8.0_91. +1 unit 0m 47s hadoop-hdfs-client in the patch passed with JDK v1.8.0_91. -1 unit 58m 18s hadoop-hdfs in the patch failed with JDK v1.8.0_91. -1 unit 6m 54s hadoop-common in the patch failed with JDK v1.7.0_95. +1 unit 0m 58s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 56m 59s hadoop-hdfs in the patch failed with JDK v1.7.0_95. -1 asflicense 0m 28s The patch generated 1 ASF License warnings. 207m 51s Reason Tests FindBugs module:hadoop-common-project/hadoop-common   Should org.apache.hadoop.fs.FileSystemStorageStatistics$LongStatisticIterator be a static inner class? At FileSystemStorageStatistics.java:inner class? At FileSystemStorageStatistics.java: [lines 51-78] JDK v1.8.0_91 Failed junit tests hadoop.fs.TestFilterFileSystem   hadoop.fs.TestHarFileSystem   hadoop.hdfs.TestFileAppend   hadoop.hdfs.TestHFlush JDK v1.7.0_95 Failed junit tests hadoop.fs.TestFilterFileSystem   hadoop.fs.TestHarFileSystem   hadoop.hdfs.TestHFlush   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12802071/HADOOP-13065.008.patch JIRA Issue HADOOP-13065 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4b827a907233 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 / ed54f5f Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/diff-checkstyle-root.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/whitespace-eol.txt findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/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/9268/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/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/9268/testReport/ asflicense https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9268/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Hi Colin P. McCabe, I was addressing the initial use case following the newly designed API. The two questions baffle me about the v8 patch. 1) How to maintain shared op->count storage statistic for all file system objects and threads. I think our use case does not need the per-FileSystem stats like S3A. My first idea was to register a single instance to the global storage statistics. 2) How to implement the single counter for each operation. As we need the atomic increment support among threads, I'm wondering how the volatile long comes into play. I agree with your previous comment that the thread local implementation is not ideal for this use case as the RPC call will generally dominate the total overhead anyway. If true, an AtomicLong would work just fine.

          Do you have any quick comments about this? Thanks.

          Show
          liuml07 Mingliang Liu added a comment - Hi Colin P. McCabe , I was addressing the initial use case following the newly designed API. The two questions baffle me about the v8 patch. 1) How to maintain shared op->count storage statistic for all file system objects and threads. I think our use case does not need the per-FileSystem stats like S3A. My first idea was to register a single instance to the global storage statistics. 2) How to implement the single counter for each operation. As we need the atomic increment support among threads, I'm wondering how the volatile long comes into play. I agree with your previous comment that the thread local implementation is not ideal for this use case as the RPC call will generally dominate the total overhead anyway. If true, an AtomicLong would work just fine. Do you have any quick comments about this? Thanks.
          Hide
          cmccabe Colin P. McCabe added a comment -

          Interesting post... I wasn't aware that AtomicLong etc. had performance issues.

          However, I don't think we need an API for updating metrics. We only need an API for reading metrics. The current read API in this patch supports reading primitive longs, which should work well with AtomicLongFieldUpdater, or whatever else we want to use.

          Show
          cmccabe Colin P. McCabe added a comment - Interesting post... I wasn't aware that AtomicLong etc. had performance issues. However, I don't think we need an API for updating metrics. We only need an API for reading metrics. The current read API in this patch supports reading primitive longs, which should work well with AtomicLongFieldUpdater , or whatever else we want to use.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Related to this, I've come across some details about how the reflection-based AtomicLongFieldUpdater is going to become a lot closer to non-atomic volatile long ++ calls: http://shipilev.net/blog/2015/faster-atomic-fu/

          This argues in favour of using this operation for updating metrics directly...it'd mean that the variables could just be simple volatile long, with a static AtomicLongFieldUpdater used to update them all

          public final class readMetrics {
          
          private static final AtomicLongFieldUpdater bytesReadUpdater = AtomicLongFieldUpdater.newUpdater(Cell.class, "bytesRead");
          private volatile long bytesRead;
          
          public long incBytesRead(long count) {
            return bytesReadUpdater.addAndGet(count);
          }
          

          HBase uses this in org.apache.hadoop.hbase.util.Counter, where it's described as "High scalable counter. Thread safe."

          Show
          stevel@apache.org Steve Loughran added a comment - Related to this, I've come across some details about how the reflection-based AtomicLongFieldUpdater is going to become a lot closer to non-atomic volatile long ++ calls: http://shipilev.net/blog/2015/faster-atomic-fu/ This argues in favour of using this operation for updating metrics directly...it'd mean that the variables could just be simple volatile long , with a static AtomicLongFieldUpdater used to update them all public final class readMetrics { private static final AtomicLongFieldUpdater bytesReadUpdater = AtomicLongFieldUpdater.newUpdater(Cell.class, "bytesRead" ); private volatile long bytesRead; public long incBytesRead( long count) { return bytesReadUpdater.addAndGet(count); } HBase uses this in org.apache.hadoop.hbase.util.Counter , where it's described as "High scalable counter. Thread safe."
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 10s 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 6m 48s trunk passed
          +1 compile 6m 9s trunk passed with JDK v1.8.0_91
          +1 compile 7m 1s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 23s trunk passed
          +1 mvnsite 1m 4s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 1m 41s trunk passed
          +1 javadoc 0m 52s trunk passed with JDK v1.8.0_91
          +1 javadoc 1m 4s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 42s the patch passed
          +1 compile 6m 11s the patch passed with JDK v1.8.0_91
          -1 javac 6m 11s root-jdk1.8.0_91 with JDK v1.8.0_91 generated 1 new + 738 unchanged - 1 fixed = 739 total (was 739)
          +1 compile 7m 0s the patch passed with JDK v1.7.0_95
          -1 javac 7m 0s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 1 new + 735 unchanged - 1 fixed = 736 total (was 736)
          -1 checkstyle 0m 24s hadoop-common-project/hadoop-common: The patch generated 32 new + 128 unchanged - 1 fixed = 160 total (was 129)
          +1 mvnsite 0m 58s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix.
          -1 findbugs 1m 55s hadoop-common-project/hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 0m 53s the patch passed with JDK v1.8.0_91
          +1 javadoc 1m 5s the patch passed with JDK v1.7.0_95
          -1 unit 6m 43s hadoop-common in the patch failed with JDK v1.8.0_91.
          -1 unit 7m 4s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 25s The patch does not generate ASF License warnings.
          60m 14s



          Reason Tests
          FindBugs module:hadoop-common-project/hadoop-common
            Should org.apache.hadoop.fs.FileSystemStorageStatistics$LongStatisticIterator be a static inner class? At FileSystemStorageStatistics.java:inner class? At FileSystemStorageStatistics.java:[lines 51-78]
          JDK v1.8.0_91 Failed junit tests hadoop.fs.TestFilterFileSystem
            hadoop.fs.TestHarFileSystem
          JDK v1.7.0_95 Failed junit tests hadoop.fs.TestFilterFileSystem
            hadoop.fs.TestHarFileSystem



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:cf2ee45
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801162/HADOOP-13065-007.patch
          JIRA Issue HADOOP-13065
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 97485597795f 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 / 7da540d
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_91.txt
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/whitespace-eol.txt
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/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/9233/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/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/9233/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/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 10s 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 6m 48s trunk passed +1 compile 6m 9s trunk passed with JDK v1.8.0_91 +1 compile 7m 1s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 23s trunk passed +1 mvnsite 1m 4s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 1m 41s trunk passed +1 javadoc 0m 52s trunk passed with JDK v1.8.0_91 +1 javadoc 1m 4s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 42s the patch passed +1 compile 6m 11s the patch passed with JDK v1.8.0_91 -1 javac 6m 11s root-jdk1.8.0_91 with JDK v1.8.0_91 generated 1 new + 738 unchanged - 1 fixed = 739 total (was 739) +1 compile 7m 0s the patch passed with JDK v1.7.0_95 -1 javac 7m 0s root-jdk1.7.0_95 with JDK v1.7.0_95 generated 1 new + 735 unchanged - 1 fixed = 736 total (was 736) -1 checkstyle 0m 24s hadoop-common-project/hadoop-common: The patch generated 32 new + 128 unchanged - 1 fixed = 160 total (was 129) +1 mvnsite 0m 58s the patch passed +1 mvneclipse 0m 13s the patch passed -1 whitespace 0m 0s The patch has 4 line(s) that end in whitespace. Use git apply --whitespace=fix. -1 findbugs 1m 55s hadoop-common-project/hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 0m 53s the patch passed with JDK v1.8.0_91 +1 javadoc 1m 5s the patch passed with JDK v1.7.0_95 -1 unit 6m 43s hadoop-common in the patch failed with JDK v1.8.0_91. -1 unit 7m 4s hadoop-common in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 25s The patch does not generate ASF License warnings. 60m 14s Reason Tests FindBugs module:hadoop-common-project/hadoop-common   Should org.apache.hadoop.fs.FileSystemStorageStatistics$LongStatisticIterator be a static inner class? At FileSystemStorageStatistics.java:inner class? At FileSystemStorageStatistics.java: [lines 51-78] JDK v1.8.0_91 Failed junit tests hadoop.fs.TestFilterFileSystem   hadoop.fs.TestHarFileSystem JDK v1.7.0_95 Failed junit tests hadoop.fs.TestFilterFileSystem   hadoop.fs.TestHarFileSystem Subsystem Report/Notes Docker Image:yetus/hadoop:cf2ee45 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12801162/HADOOP-13065-007.patch JIRA Issue HADOOP-13065 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 97485597795f 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 / 7da540d Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_91 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/diff-compile-javac-root-jdk1.8.0_91.txt javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/diff-compile-javac-root-jdk1.7.0_95.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/whitespace-eol.txt findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/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/9233/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_91.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/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/9233/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9233/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cmccabe Colin P. McCabe added a comment -

          Thanks, Mingliang Liu.

          Based on the discussion today, it sounds like we would like to have both global statistics per FS class, and per-instance statistics for an individual FS or FC instance. The rationale for this is that in some cases we might want to differentiate between, say, the stats when talking to one s3 bucket, and another s3 bucket. Or another example is the stats talking to one HDFS FS versus another HDFS FS (if we are using federation, or just multiple HDFS instances).

          We talked a bit about metrics2, but there were several things that made it not a good fit for this statistics interface. One issue is that metrics2 assumes that statistics are permanent once created. Effectively, it keeps them around until the JVM terminates. metrics2 also tends to use a fair amount of memory and require a fair amount of boilerplate code compared to other solutions. Finally, because it is global, it can't do per-instance stats very effectively.

          It would be nice for the new statistics interface to provide the same stats which are currently provided by FileSystem#Statistics. This would allow us to deprecate and eventually remove FileSystem#Statistics as a public interface (although we might keep the implementation). This could be done only in a new release of Hadoop, of course. We also talked about the benefits of providing an iterator over all statistics rather than a map of all statistics. Relatedly, we talked about the desire to have a new interface that was abstract enough to accommodate new, more efficient implementations in the future.

          For now, the new interface will deal with per-FS stats, but not per-stream ones. We should revisit per-stream statistics later.

          Show
          cmccabe Colin P. McCabe added a comment - Thanks, Mingliang Liu . Based on the discussion today, it sounds like we would like to have both global statistics per FS class, and per-instance statistics for an individual FS or FC instance. The rationale for this is that in some cases we might want to differentiate between, say, the stats when talking to one s3 bucket, and another s3 bucket. Or another example is the stats talking to one HDFS FS versus another HDFS FS (if we are using federation, or just multiple HDFS instances). We talked a bit about metrics2, but there were several things that made it not a good fit for this statistics interface. One issue is that metrics2 assumes that statistics are permanent once created. Effectively, it keeps them around until the JVM terminates. metrics2 also tends to use a fair amount of memory and require a fair amount of boilerplate code compared to other solutions. Finally, because it is global, it can't do per-instance stats very effectively. It would be nice for the new statistics interface to provide the same stats which are currently provided by FileSystem#Statistics. This would allow us to deprecate and eventually remove FileSystem#Statistics as a public interface (although we might keep the implementation). This could be done only in a new release of Hadoop, of course. We also talked about the benefits of providing an iterator over all statistics rather than a map of all statistics. Relatedly, we talked about the desire to have a new interface that was abstract enough to accommodate new, more efficient implementations in the future. For now, the new interface will deal with per-FS stats, but not per-stream ones. We should revisit per-stream statistics later.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 docker 0m 3s Docker failed to build yetus/hadoop:7b1c37a.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12799860/HDFS-10175.006.patch
          JIRA Issue HADOOP-13065
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9194/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 0s Docker mode activated. -1 docker 0m 3s Docker failed to build yetus/hadoop:7b1c37a. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12799860/HDFS-10175.006.patch JIRA Issue HADOOP-13065 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9194/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Per offline discussion, moved from HDFS project to Hadoop Common project.

          Show
          liuml07 Mingliang Liu added a comment - Per offline discussion, moved from HDFS project to Hadoop Common project.
          Hide
          cmccabe Colin P. McCabe added a comment -

          BTW, sorry for the last-minute-ness of this scheduling, Mingliang Liu and Steve Loughran.

          Webex here at 10:30:

          HDFS-10175 webex
          Wednesday, April 27, 2016
          10:30 am | Pacific Daylight Time (San Francisco, GMT-07:00) | 1 hr

          JOIN WEBEX MEETING
          https://cloudera.webex.com/cloudera/j.php?MTID=mebca25435f158dec71b2589561e71b29
          Meeting number: 294 963 170 Meeting password: 1234

          JOIN BY PHONE 1-650-479-3208 Call-in toll number (US/Canada) Access code: 294 963 170 Global call-in numbers: https://cloudera.webex.com/cloudera/globalcallin.php?serviceType=MC&ED=45642173&tollFree=0

          Show
          cmccabe Colin P. McCabe added a comment - BTW, sorry for the last-minute-ness of this scheduling, Mingliang Liu and Steve Loughran . Webex here at 10:30: HDFS-10175 webex Wednesday, April 27, 2016 10:30 am | Pacific Daylight Time (San Francisco, GMT-07:00) | 1 hr JOIN WEBEX MEETING https://cloudera.webex.com/cloudera/j.php?MTID=mebca25435f158dec71b2589561e71b29 Meeting number: 294 963 170 Meeting password: 1234 JOIN BY PHONE 1-650-479-3208 Call-in toll number (US/Canada) Access code: 294 963 170 Global call-in numbers: https://cloudera.webex.com/cloudera/globalcallin.php?serviceType=MC&ED=45642173&tollFree=0
          Hide
          cmccabe Colin P. McCabe added a comment -

          Great. Let me add a webex

          Show
          cmccabe Colin P. McCabe added a comment - Great. Let me add a webex
          Hide
          stevel@apache.org Steve Loughran added a comment -

          One thing that is a bit concerning about metrics2 is that I think people feel that this interface should be stable (i.e. don't remove or alter things once they're in), which would be a big constraint on us.

          Ops teams don't like metrics they rely on being taken away; they also view published metrics as the API. As the compatibility docs say "Metrics should preserve compatibility within the major release."

          Perhaps we could document that per-fs stats were @Public @Evolving rather than stable

          +1 to that, though it'll be important not to break binary compatibility with external filesystems.

          FWIW, issues I have with metrics2, apart from "steve doesn't understand the design fully" is its preference for singletons registered with JMX. This makes sense in deployed services, not for tests.

          Do we have any ideas about how Spark will consume these metrics in the longer term?

          Spark ia a Coda Hale instrumented codebase, as are many other apps these days. Integration between hadoop metrics of any form and the Coda Hale libraries would be something to address, but not here.

          Show
          stevel@apache.org Steve Loughran added a comment - One thing that is a bit concerning about metrics2 is that I think people feel that this interface should be stable (i.e. don't remove or alter things once they're in), which would be a big constraint on us. Ops teams don't like metrics they rely on being taken away; they also view published metrics as the API. As the compatibility docs say "Metrics should preserve compatibility within the major release." Perhaps we could document that per-fs stats were @Public @Evolving rather than stable +1 to that, though it'll be important not to break binary compatibility with external filesystems. FWIW, issues I have with metrics2, apart from "steve doesn't understand the design fully" is its preference for singletons registered with JMX. This makes sense in deployed services, not for tests. Do we have any ideas about how Spark will consume these metrics in the longer term? Spark ia a Coda Hale instrumented codebase, as are many other apps these days. Integration between hadoop metrics of any form and the Coda Hale libraries would be something to address, but not here.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Wednesday april 26, 10:30 AM PST; 18:30 UK, 17:30 GMT works for me.

          Webex binding?

          if you don't specify one, mine is at https://hortonworks.webex.com/meet/stevel

          Show
          stevel@apache.org Steve Loughran added a comment - Wednesday april 26, 10:30 AM PST; 18:30 UK, 17:30 GMT works for me. Webex binding? if you don't specify one, mine is at https://hortonworks.webex.com/meet/stevel
          Hide
          liuml07 Mingliang Liu added a comment -

          Either tomorrow or next week will work for me. Thanks.

          Show
          liuml07 Mingliang Liu added a comment - Either tomorrow or next week will work for me. Thanks.
          Hide
          cmccabe Colin P. McCabe added a comment -

          Hi Steve Loughran, does 10:30AM work tomorrow? Unfortunately I'll be out on Thursday and most of Friday, so if we can't do tomorrow we'd have to do Friday afternoon or early next week.

          Show
          cmccabe Colin P. McCabe added a comment - Hi Steve Loughran , does 10:30AM work tomorrow? Unfortunately I'll be out on Thursday and most of Friday, so if we can't do tomorrow we'd have to do Friday afternoon or early next week.
          Hide
          cmccabe Colin P. McCabe added a comment -

          I see that, but stream-level counters are essential at least for the tests which verify forward and lazy seeks. Which means that yes, they do have to go into the 2.8.0 release. What I can do is set up the scope so that they are package private, then, in the test code, implement the assertions about metric-derived state into that package.

          I guess my hope here is that whatever mechanism we come up with is something that could easily be integrated into the upcoming 2.8 release. Since we have talked about requiring our new metrics to not modify existing stable public interfaces, that seems very reasonable.

          One thing that is a bit concerning about metrics2 is that I think people feel that this interface should be stable (i.e. don't remove or alter things once they're in), which would be a big constraint on us. Perhaps we could document that per-fs stats were @Public @Evolving rather than stable?

          Regarding the metrics2 instrumentation in HADOOP-13028, I'm aggregating the stream statistics back into the metrics 2 data. That's something which isn't needed for the hadoop tests, but which I'm logging in spark test runs, such as (formatted for readability):

          Do we have any ideas about how Spark will consume these metrics in the longer term? Do they prefer to go through metrics2, for example? I definitely don't object to putting this kind of stuff in metrics2, but if we go that route, we have to accept that we'll just get global (or at best per-fs-type) statistics, rather than per-fs-instance statistics. Is that acceptable? So far, nobody has spoken up strongly in favor of per-fs-instance statistics.

          Show
          cmccabe Colin P. McCabe added a comment - I see that, but stream-level counters are essential at least for the tests which verify forward and lazy seeks. Which means that yes, they do have to go into the 2.8.0 release. What I can do is set up the scope so that they are package private, then, in the test code, implement the assertions about metric-derived state into that package. I guess my hope here is that whatever mechanism we come up with is something that could easily be integrated into the upcoming 2.8 release. Since we have talked about requiring our new metrics to not modify existing stable public interfaces, that seems very reasonable. One thing that is a bit concerning about metrics2 is that I think people feel that this interface should be stable (i.e. don't remove or alter things once they're in), which would be a big constraint on us. Perhaps we could document that per-fs stats were @Public @Evolving rather than stable? Regarding the metrics2 instrumentation in HADOOP-13028 , I'm aggregating the stream statistics back into the metrics 2 data. That's something which isn't needed for the hadoop tests, but which I'm logging in spark test runs, such as (formatted for readability): Do we have any ideas about how Spark will consume these metrics in the longer term? Do they prefer to go through metrics2, for example? I definitely don't object to putting this kind of stuff in metrics2, but if we go that route, we have to accept that we'll just get global (or at best per-fs-type) statistics, rather than per-fs-instance statistics. Is that acceptable? So far, nobody has spoken up strongly in favor of per-fs-instance statistics.
          Hide
          cmccabe Colin P. McCabe added a comment -

          I prefer earlier (being in UK time and all); I could do the first half hour of the webex [at 12:30pm]

          How about 10:30AM PST to noon on tomorrow on Wednesday?

          Show
          cmccabe Colin P. McCabe added a comment - I prefer earlier (being in UK time and all); I could do the first half hour of the webex [at 12:30pm] How about 10:30AM PST to noon on tomorrow on Wednesday?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          One piece of background here is what I'm currently exploring in terms of Metrics-first testing, that is, instrumenting code and using it's observed state in both unit and system tests. I've done in in Slider (SLIDER-82) and spark (SPARK-7889) and found it highly effective for writing deterministic tests which also provide information parseable by test runners for better analysis of test runs.

          I understand your eagerness to get the s3 stats in, but I would rather not proliferate more statistics interfaces if possible. Once they're in, we really can't get rid of them, and it becomes very confusing and clunky.

          I see that, but stream-level counters are essential at least for the tests which verify forward and lazy seeks. Which means that yes, they do have to go into the 2.8.0 release. What I can do is set up the scope so that they are package private, then, in the test code, implement the assertions about metric-derived state into that package.

          Regarding the metrics2 instrumentation in HADOOP-13028, I'm aggregating the stream statistics back into the metrics 2 data. That's something which isn't needed for the hadoop tests, but which I'm logging in spark test runs, such as (formatted for readability):

          2016-04-26 12:08:25,901  executor.Executor Running task 0.0 in stage 0.0 (TID 0)
          2016-04-26 12:08:25,924  rdd.HadoopRDD Input split: s3a://landsat-pds/scene_list.gz:0+20430493
          2016-04-26 12:08:26,107  compress.CodecPool - Got brand-new decompressor [.gz]
          2016-04-26 12:08:32,304  executor.Executor Finished task 0.0 in stage 0.0 (TID 0). 
            2643 bytes result sent to driver
          2016-04-26 12:08:32,311  scheduler.TaskSetManager Finished task 0.0 in stage 0.0 (TID 0)
            in 6434 ms on localhost (1/1)
          2016-04-26 12:08:32,312  scheduler.TaskSchedulerImpl Removed TaskSet 0.0, whose tasks
            have all completed, from pool 
          2016-04-26 12:08:32,315  scheduler.DAGScheduler ResultStage 0 finished in 6.447 s
          2016-04-26 12:08:32,319  scheduler.DAGScheduler Job 0 finished took 6.560166 s
          2016-04-26 12:08:32,320  s3.S3aIOSuite  size of s3a://landsat-pds/scene_list.gz = 464105
            rows read in 6779125000 nS
          
          2016-04-26 12:08:32,324 s3.S3aIOSuite Filesystem statistics
            S3AFileSystem{uri=s3a://landsat-pds,
            workingDir=s3a://landsat-pds/user/stevel,
            partSize=104857600, enableMultiObjectsDelete=true,
            multiPartThreshold=2147483647,
            statistics {
              20430493 bytes read,
               0 bytes written,
               3 read ops,
               0 large read ops,
               0 write ops},
               metrics {{Context=S3AFileSystem}
                {FileSystemId=29890500-aed6-4eb8-bb47-0c896a66aac2-landsat-pds}
                {fsURI=s3a://landsat-pds/scene_list.gz}
                {streamOpened=1}
                {streamCloseOperations=1}
                {streamClosed=1}
                {streamAborted=0}
                {streamSeekOperations=0}
                {streamReadExceptions=0}
                {streamForwardSeekOperations=0}
                {streamBackwardSeekOperations=0}
                {streamBytesSkippedOnSeek=0}
                {streamBytesRead=20430493}
                {streamReadOperations=1488}
                {streamReadFullyOperations=0}
                {streamReadOperationsIncomplete=1488}
                {files_created=0}
                {files_copied=0}
                {files_copied_bytes=0}
                {files_deleted=0}
                {directories_created=0}
                {directories_deleted=0}
                {ignored_errors=0} 
                }}
          

          The spark code isn't accessing these metrics, though it could if it tried hard (went to the metric registry).

          It's publishing those stream level operations which I think you are most concerned about; the other metrics are roughly a subset of those already in Azure's metrics2 instrumentation. Accordingly, I will modify the S3 instrumentation to not register the stream operations as metrics2 counters, retaining them internally and in the toString value.

          I hope that's enough to satisfy your concerns while still retaining the information I need in s3a functionality and testing

          Show
          stevel@apache.org Steve Loughran added a comment - One piece of background here is what I'm currently exploring in terms of Metrics-first testing , that is, instrumenting code and using it's observed state in both unit and system tests. I've done in in Slider ( SLIDER-82 ) and spark ( SPARK-7889 ) and found it highly effective for writing deterministic tests which also provide information parseable by test runners for better analysis of test runs. I understand your eagerness to get the s3 stats in, but I would rather not proliferate more statistics interfaces if possible. Once they're in, we really can't get rid of them, and it becomes very confusing and clunky. I see that, but stream-level counters are essential at least for the tests which verify forward and lazy seeks. Which means that yes, they do have to go into the 2.8.0 release. What I can do is set up the scope so that they are package private, then, in the test code, implement the assertions about metric-derived state into that package. Regarding the metrics2 instrumentation in HADOOP-13028 , I'm aggregating the stream statistics back into the metrics 2 data. That's something which isn't needed for the hadoop tests, but which I'm logging in spark test runs, such as (formatted for readability): 2016-04-26 12:08:25,901 executor.Executor Running task 0.0 in stage 0.0 (TID 0) 2016-04-26 12:08:25,924 rdd.HadoopRDD Input split: s3a: //landsat-pds/scene_list.gz:0+20430493 2016-04-26 12:08:26,107 compress.CodecPool - Got brand- new decompressor [.gz] 2016-04-26 12:08:32,304 executor.Executor Finished task 0.0 in stage 0.0 (TID 0). 2643 bytes result sent to driver 2016-04-26 12:08:32,311 scheduler.TaskSetManager Finished task 0.0 in stage 0.0 (TID 0) in 6434 ms on localhost (1/1) 2016-04-26 12:08:32,312 scheduler.TaskSchedulerImpl Removed TaskSet 0.0, whose tasks have all completed, from pool 2016-04-26 12:08:32,315 scheduler.DAGScheduler ResultStage 0 finished in 6.447 s 2016-04-26 12:08:32,319 scheduler.DAGScheduler Job 0 finished took 6.560166 s 2016-04-26 12:08:32,320 s3.S3aIOSuite size of s3a: //landsat-pds/scene_list.gz = 464105 rows read in 6779125000 nS 2016-04-26 12:08:32,324 s3.S3aIOSuite Filesystem statistics S3AFileSystem{uri=s3a: //landsat-pds, workingDir=s3a: //landsat-pds/user/stevel, partSize=104857600, enableMultiObjectsDelete= true , multiPartThreshold=2147483647, statistics { 20430493 bytes read, 0 bytes written, 3 read ops, 0 large read ops, 0 write ops}, metrics {{Context=S3AFileSystem} {FileSystemId=29890500-aed6-4eb8-bb47-0c896a66aac2-landsat-pds} {fsURI=s3a: //landsat-pds/scene_list.gz} {streamOpened=1} {streamCloseOperations=1} {streamClosed=1} {streamAborted=0} {streamSeekOperations=0} {streamReadExceptions=0} {streamForwardSeekOperations=0} {streamBackwardSeekOperations=0} {streamBytesSkippedOnSeek=0} {streamBytesRead=20430493} {streamReadOperations=1488} {streamReadFullyOperations=0} {streamReadOperationsIncomplete=1488} {files_created=0} {files_copied=0} {files_copied_bytes=0} {files_deleted=0} {directories_created=0} {directories_deleted=0} {ignored_errors=0} }} The spark code isn't accessing these metrics, though it could if it tried hard (went to the metric registry). It's publishing those stream level operations which I think you are most concerned about; the other metrics are roughly a subset of those already in Azure's metrics2 instrumentation. Accordingly, I will modify the S3 instrumentation to not register the stream operations as metrics2 counters, retaining them internally and in the toString value. I hope that's enough to satisfy your concerns while still retaining the information I need in s3a functionality and testing
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I prefer earlier (being in UK time and all); I could do the first half hour of the webex

          Show
          stevel@apache.org Steve Loughran added a comment - I prefer earlier (being in UK time and all); I could do the first half hour of the webex
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks for proposing the discussion. The time works for me.

          Show
          liuml07 Mingliang Liu added a comment - Thanks for proposing the discussion. The time works for me.
          Hide
          cmccabe Colin P. McCabe added a comment -

          We already have three statistics interfaces:
          1. FileSystem#Statistics
          2. DFSInputStream#ReadStatistics
          3. metrics2 etc.

          #1 existed for a very long time and is tied into MR in the ways discussed above. I didn't create it, but I did implement the thread-local optimization, based on some performance issues we were having.

          I have to take the blame for adding #2, in HDFS-4698. At the time, the main focus was on ensuring we were doing short-circuit reads, which didn't really fit into #1. And like you, I felt that it was "very low-level stream behavior" that was decoupled from the rest of the stats.

          Of course #3 has been around a while, and is used more generally than just in our storage code.

          I understand your eagerness to get the s3 stats in, but I would rather not proliferate more statistics interfaces if possible. Once they're in, we really can't get rid of them, and it becomes very confusing and clunky.

          Are you guys free for a webex on Wednesday afternoon? Maybe 12:30pm to 2pm?

          Show
          cmccabe Colin P. McCabe added a comment - We already have three statistics interfaces: 1. FileSystem#Statistics 2. DFSInputStream#ReadStatistics 3. metrics2 etc. #1 existed for a very long time and is tied into MR in the ways discussed above. I didn't create it, but I did implement the thread-local optimization, based on some performance issues we were having. I have to take the blame for adding #2, in HDFS-4698 . At the time, the main focus was on ensuring we were doing short-circuit reads, which didn't really fit into #1. And like you, I felt that it was "very low-level stream behavior" that was decoupled from the rest of the stats. Of course #3 has been around a while, and is used more generally than just in our storage code. I understand your eagerness to get the s3 stats in, but I would rather not proliferate more statistics interfaces if possible. Once they're in, we really can't get rid of them, and it becomes very confusing and clunky. Are you guys free for a webex on Wednesday afternoon? Maybe 12:30pm to 2pm?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          yes, could have a talk. I actually thinks this is both related to and different from the s3a stuff... that is, it can go in without this, as it is currently more on some overall metrics and some very low level counters for testing stream behaviour (rather than for direct reporting, though that could come)

          1. some JMX-able stats for each s3a instance (taken from Azure, not per-thread)
          2. per-stream stats (volatile, merged with s3a stats above when stream is close()d
          3. tests to verify behaviour of the FS (e.g. lazy seek doesn't open files, read-ahead skips bytes, etc)

          I agree about consistency; for the stream ones I've done the fields are tagged as volatile. These are merged into the JMX-layer instrumentation at close time, so sync costs are low

          Show
          stevel@apache.org Steve Loughran added a comment - yes, could have a talk. I actually thinks this is both related to and different from the s3a stuff... that is, it can go in without this, as it is currently more on some overall metrics and some very low level counters for testing stream behaviour (rather than for direct reporting, though that could come) some JMX-able stats for each s3a instance (taken from Azure, not per-thread) per-stream stats (volatile, merged with s3a stats above when stream is close()d tests to verify behaviour of the FS (e.g. lazy seek doesn't open files, read-ahead skips bytes, etc) I agree about consistency; for the stream ones I've done the fields are tagged as volatile. These are merged into the JMX-layer instrumentation at close time, so sync costs are low
          Hide
          cmccabe Colin P. McCabe added a comment - - edited

          Thanks for commenting, Steve Loughran. It's great to see work on s3 stats. s3a has needed love for a while.

          Did you get a chance to look at my HDFS-10175.006.patch on this JIRA? It seems to address all of your concerns. It provides a standard API that every FileSystem can implement (not just s3, just HDFS, etc. etc.). Once we adopt jdk8, we can easily implement this API using java.util.concurrent.atomic.LongAdder if that proves to be more readable and/or efficient.

          Don't break any existing filesystem code by adding new params to existing methods, etc.

          I agree. My patch doesn't add new params to any existing methods.

          add the new code out of FileSystem

          I agree. That's why I separated StorageStatistics.java from FileSystem.java. FileContext should be able to use this API as well, simply by returning a StorageStatistics instance just like FileSystem does.

          Use an int rather than an enum; lets filesystems add their own counters. I hereby reserve 0x200-0x255 for object store operations.

          Hmm. I'm not sure I follow. My patch identifies counters by name (string), not by an int, enum, or byte. This is necessary because different storage backends will want to track different things (s3a wants to track s3 PUTs, HDFS wants to track genstamp bump ops, etc. etc.). We should not try to create the "statistics type enum of doom" in some misguided attempt at space optimization. Consider the case of out-of-tree Filesystem implementations as well... they are not going to add entries to some enum of doom in hadoop-common.

          public interface StatisticsSource { Map<String, Long> snapshot(); }

          I don't think an API that returns a map is the right approach for statistics. That map could get quite large. We already know that people love adding just one more statistic to things (and often for quite valid reasons). It's very difficult to -1 a patch just because it bloats the statistics map more. Once this API exists, the natural progression will be people adding tons and tons of new entries to it. We should be prepared for this and use an API that doesn't choke if we have tons of stats. We shouldn't have to materialize everything all the time-- an iterator approach is smarter because it can be O(1) in terms of memory, no matter how many entries we have.

          I also don't think we need snapshot consistency for stats. It's a heavy burden for an implementation to carry (it basically requires some kind of materialization into a map, and probably synchronization to stop the world while the materialization is going on). And there is no user demand for it... the current FileSystem#Statistics interface doesn't have it, and nobody has asked for it.

          It seems like you are focusing on the ability to expose new stats to our metrics2 subsystem, while this JIRA originally focused on adding metrics that MapReduce could read at the end of a job. I think these two use-cases can be covered by the same API. We should try to hammer that out (probably as a HADOOP JIRA rather than an HDFS JIRA, as well).

          Do you think we should have a call about this or something? I know some folks who might be interested in testing the s3 metrics stuff, if there was a reasonable API to access it.

          Show
          cmccabe Colin P. McCabe added a comment - - edited Thanks for commenting, Steve Loughran . It's great to see work on s3 stats. s3a has needed love for a while. Did you get a chance to look at my HDFS-10175 .006.patch on this JIRA? It seems to address all of your concerns. It provides a standard API that every FileSystem can implement (not just s3, just HDFS, etc. etc.). Once we adopt jdk8, we can easily implement this API using java.util.concurrent.atomic.LongAdder if that proves to be more readable and/or efficient. Don't break any existing filesystem code by adding new params to existing methods, etc. I agree. My patch doesn't add new params to any existing methods. add the new code out of FileSystem I agree. That's why I separated StorageStatistics.java from FileSystem.java . FileContext should be able to use this API as well, simply by returning a StorageStatistics instance just like FileSystem does. Use an int rather than an enum; lets filesystems add their own counters. I hereby reserve 0x200-0x255 for object store operations. Hmm. I'm not sure I follow. My patch identifies counters by name (string), not by an int, enum, or byte. This is necessary because different storage backends will want to track different things (s3a wants to track s3 PUTs, HDFS wants to track genstamp bump ops, etc. etc.). We should not try to create the "statistics type enum of doom" in some misguided attempt at space optimization. Consider the case of out-of-tree Filesystem implementations as well... they are not going to add entries to some enum of doom in hadoop-common. public interface StatisticsSource { Map<String, Long> snapshot(); } I don't think an API that returns a map is the right approach for statistics. That map could get quite large. We already know that people love adding just one more statistic to things (and often for quite valid reasons). It's very difficult to -1 a patch just because it bloats the statistics map more. Once this API exists, the natural progression will be people adding tons and tons of new entries to it. We should be prepared for this and use an API that doesn't choke if we have tons of stats. We shouldn't have to materialize everything all the time-- an iterator approach is smarter because it can be O(1) in terms of memory, no matter how many entries we have. I also don't think we need snapshot consistency for stats. It's a heavy burden for an implementation to carry (it basically requires some kind of materialization into a map, and probably synchronization to stop the world while the materialization is going on). And there is no user demand for it... the current FileSystem#Statistics interface doesn't have it, and nobody has asked for it. It seems like you are focusing on the ability to expose new stats to our metrics2 subsystem, while this JIRA originally focused on adding metrics that MapReduce could read at the end of a job. I think these two use-cases can be covered by the same API. We should try to hammer that out (probably as a HADOOP JIRA rather than an HDFS JIRA, as well). Do you think we should have a call about this or something? I know some folks who might be interested in testing the s3 metrics stuff, if there was a reasonable API to access it.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          +one more thing that would be nice would be for all classes which collect stats to implement an interface to publish those stats:

          pubic interface StatisticsSource {
            Map<String, Long> snapshot();
          }
          

          Why? It's something which filesystems subclasses can implement, IO Streams, etc. The FSData streams would implement and pass on to wrapped streams, or return an empty map.

          Having a uniform mech like this would make it easy for tests to grab the before/after data, look for diffs, display counts, etc. By avoiding any enums in the returned snapshot, it'd be easy for applications that aren't built against a specific Hadoop version (Spark, etc) to grab all stats available to the app, without knowing the exact mappings, and present them to users in reports .

          Show
          stevel@apache.org Steve Loughran added a comment - +one more thing that would be nice would be for all classes which collect stats to implement an interface to publish those stats: pubic interface StatisticsSource { Map< String , Long > snapshot(); } Why? It's something which filesystems subclasses can implement, IO Streams, etc. The FSData streams would implement and pass on to wrapped streams, or return an empty map. Having a uniform mech like this would make it easy for tests to grab the before/after data, look for diffs, display counts, etc. By avoiding any enums in the returned snapshot, it'd be easy for applications that aren't built against a specific Hadoop version (Spark, etc) to grab all stats available to the app, without knowing the exact mappings, and present them to users in reports .
          Hide
          stevel@apache.org Steve Loughran added a comment -
          1. Don't break any existing filesystem code by adding new params to existing methods, etc.
          2. add the new code out of FileSystem
          3. Use an int rather than an enum; lets filesystems add their own counters. I hereby reserve 0x200-0x255 for object store operations.

          With an open int rather than an enum, the map size is dependent upon the active ops, not the possible set. An initial hashmap using the int value as key should work, maybe set the default capacity to that of the "standard" FS ops. entry creation would have to be on demand.

          Alternatively, do fix the #of operations at compile time, and store in an array of volatile[], so per-thread storage is 4 bytes * op * thread, lookup O(1). With the 46 opcodes in the patch, that's 184 bytes/fs/thread.

          Here the increment operation returns the new value of -1 for either of : no logging, no such opcode. An out of range opcode has costs of exception raising; no counters is probability and penalty of speculation prediction failure.

          public long inc(opcode, count) {
            try {
             return counters !=null ?  counters[opcode]+=count : 0;
            } catch(ArrayOutOfBoundsException e) {
              return -1;
            }
          }
          

          In this situation, the #of opcodes is fixed in the hadoop version; I'll just pre-reserve some of the values for object store operations.

          Show
          stevel@apache.org Steve Loughran added a comment - Don't break any existing filesystem code by adding new params to existing methods, etc. add the new code out of FileSystem Use an int rather than an enum; lets filesystems add their own counters. I hereby reserve 0x200-0x255 for object store operations. With an open int rather than an enum, the map size is dependent upon the active ops, not the possible set. An initial hashmap using the int value as key should work, maybe set the default capacity to that of the "standard" FS ops. entry creation would have to be on demand. Alternatively, do fix the #of operations at compile time, and store in an array of volatile[], so per-thread storage is 4 bytes * op * thread, lookup O(1). With the 46 opcodes in the patch, that's 184 bytes/fs/thread. Here the increment operation returns the new value of -1 for either of : no logging, no such opcode. An out of range opcode has costs of exception raising; no counters is probability and penalty of speculation prediction failure. public long inc(opcode, count) { try { return counters != null ? counters[opcode]+=count : 0; } catch (ArrayOutOfBoundsException e) { return -1; } } In this situation, the #of opcodes is fixed in the hadoop version; I'll just pre-reserve some of the values for object store operations.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Yes, contention is an issue, especially against filesystems which respond fasts. But contention is not mandatory.

          Coda Hale counters use a com.codahale.metrics.LongAdder class which queues up addition ops under load so threads don't block:

          Under low update contention, the two classes have similar characteristics. But under high contention, expected throughput of this class is significantly higher, at the expense of higher space consumption.

          This class is now built in to Java 8 as java.util.concurrent.atomic.LongAdder alongside java.util.concurrent.atomic.LongAccumulator.

          Even with code built against Java 7, whatever is done here should be designed so that the switch to java 8 should be seamless and transparent. That is: the specific counter implementation hidden. I'd almost advocate using the coda hale one except that it would add a new dependency everywhere; it's only used in a couple of modules right now. And for trunk we may as well switch to Java 8.

          (life would be so much easier of volatiles implemented atomic add/inc ops the way CPUs allow)

          Show
          stevel@apache.org Steve Loughran added a comment - Yes, contention is an issue, especially against filesystems which respond fasts. But contention is not mandatory. Coda Hale counters use a com.codahale.metrics.LongAdder class which queues up addition ops under load so threads don't block: Under low update contention, the two classes have similar characteristics. But under high contention, expected throughput of this class is significantly higher, at the expense of higher space consumption. This class is now built in to Java 8 as java.util.concurrent.atomic.LongAdder alongside java.util.concurrent.atomic.LongAccumulator . Even with code built against Java 7, whatever is done here should be designed so that the switch to java 8 should be seamless and transparent. That is: the specific counter implementation hidden. I'd almost advocate using the coda hale one except that it would add a new dependency everywhere; it's only used in a couple of modules right now. And for trunk we may as well switch to Java 8. (life would be so much easier of volatiles implemented atomic add/inc ops the way CPUs allow)
          Hide
          cmccabe Colin P. McCabe added a comment -

          Can I also note that as the @Public @Stable FileSystem is widely subclassed, with its protected statistics field accessed in those subclasses, nobody is allowed to take it or its current methods away. Thanks.

          Yeah, I agree. I would like to see us get more cautious about adding new things to FileSystem#Statistics, though, since I think it's not a good match for most of the new stats we're proposing here.

          There's no per-thread tracking, —its collecting overall stats, rather than trying to add up the cost of a single execution, which is what per-thread stuff would, presumably do. This is lower cost but still permits microbenchmark-style analysis of performance problems against S3a. It doesn't directly let you get results of a job, "34MB of data, 2000 stream aborts, 1998 backward seeks" which are the kind of things I'm curious about.

          Overall stats are lower cost in terms of memory consumption, and the cost to read (as opposed to update) a metric. They are higher cost in terms of the CPU consumed for each update of the metric. In particular, for applications that do a lot of stream operations from many different threads, updating an AtomicLong can become a performance bottleneck

          One of the points that I was making above is that I think it's appropriate for some metrics to be tracked per-thread, but for others, we probably want to use AtomicLong or similar. I would expect that anything that led to an s3 RPC would be appropriate to be tracked by an AtomicLong very easily, since the overhead of the network activity would dwarf the AtomicLong update overhead. And we should have a common interface for getting this information that MR and stats consumers can use.

          Maybe, and this would be nice, whatever is implemented here is (a) extensible to support some duration type too, at least in parallel,

          The interface here supports storing durations as 64-bit numbers of milliseconds, which seems good. It is up to the implementor of the statistic to determine what the 64-bit long represents (average duration in ms, median duration in ms, number of RPCs, etc. etc.) This is similar to metrics2 and jmx, etc. where you have basic types that can be used in a few different ways.

          and (b) could be used as a back end by both Metrics2 and Coda Hale metrics registries. That way the slightly more expensive metric systems would have access to this more raw data.

          Sure. The difficult question is how metrics2 hooks up to metrics which are per FS or per-stream. Should the output of metrics2 reflect the union of all existing FS and stream instances? Some applications open a very large number of streams, so it seems impractical for metrics2 to include all those streams in its output.

          Show
          cmccabe Colin P. McCabe added a comment - Can I also note that as the @Public @Stable FileSystem is widely subclassed, with its protected statistics field accessed in those subclasses, nobody is allowed to take it or its current methods away. Thanks. Yeah, I agree. I would like to see us get more cautious about adding new things to FileSystem#Statistics , though, since I think it's not a good match for most of the new stats we're proposing here. There's no per-thread tracking, —its collecting overall stats, rather than trying to add up the cost of a single execution, which is what per-thread stuff would, presumably do. This is lower cost but still permits microbenchmark-style analysis of performance problems against S3a. It doesn't directly let you get results of a job, "34MB of data, 2000 stream aborts, 1998 backward seeks" which are the kind of things I'm curious about. Overall stats are lower cost in terms of memory consumption, and the cost to read (as opposed to update) a metric. They are higher cost in terms of the CPU consumed for each update of the metric. In particular, for applications that do a lot of stream operations from many different threads, updating an AtomicLong can become a performance bottleneck One of the points that I was making above is that I think it's appropriate for some metrics to be tracked per-thread, but for others, we probably want to use AtomicLong or similar. I would expect that anything that led to an s3 RPC would be appropriate to be tracked by an AtomicLong very easily, since the overhead of the network activity would dwarf the AtomicLong update overhead. And we should have a common interface for getting this information that MR and stats consumers can use. Maybe, and this would be nice, whatever is implemented here is (a) extensible to support some duration type too, at least in parallel, The interface here supports storing durations as 64-bit numbers of milliseconds, which seems good. It is up to the implementor of the statistic to determine what the 64-bit long represents (average duration in ms, median duration in ms, number of RPCs, etc. etc.) This is similar to metrics2 and jmx, etc. where you have basic types that can be used in a few different ways. and (b) could be used as a back end by both Metrics2 and Coda Hale metrics registries. That way the slightly more expensive metric systems would have access to this more raw data. Sure. The difficult question is how metrics2 hooks up to metrics which are per FS or per-stream. Should the output of metrics2 reflect the union of all existing FS and stream instances? Some applications open a very large number of streams, so it seems impractical for metrics2 to include all those streams in its output.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Can I also note that as the @Public @Stable FileSystem is widely subclassed, with its protected statistics field accessed in those subclasses, nobody is allowed to take it or its current methods away. Thanks.

          Show
          stevel@apache.org Steve Loughran added a comment - Can I also note that as the @Public @Stable FileSystem is widely subclassed, with its protected statistics field accessed in those subclasses, nobody is allowed to take it or its current methods away. Thanks.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          In HADOOP-13028 I've just implemented something for S3A lifted out of Azure's Metrics2 integration; essentially I've got a per-FS instance set of metrics2 netrucs, which are incremented in the FS or input stream as things happen.

          There's no per-thread tracking, —its collecting overall stats, rather than trying to add up the cost of a single execution, which is what per-thread stuff would, presumably do. This is lower cost but still permits microbenchmark-style analysis of performance problems against S3a. It doesn't directly let you get results of a job, "34MB of data, 2000 stream aborts, 1998 backward seeks" which are the kind of things I'm curious about.

          I think adding some duration tracking for the blobstore ops would be good too; I used that in [Swift] where it helped show that one of the public endpoints was throttling delete calls, so timing out tests.

          Again, that points more to the classic metrics stuff, or, even better Coda Hale histograms

          Maybe, and this would be nice, whatever is implemented here is (a) extensible to support some duration type too, at least in parallel, and (b) could be used as a back end by both Metrics2 and Coda Hale metrics registries. That way the slightly more expensive metric systems would have access to this more raw data.

          Show
          stevel@apache.org Steve Loughran added a comment - In HADOOP-13028 I've just implemented something for S3A lifted out of Azure's Metrics2 integration; essentially I've got a per-FS instance set of metrics2 netrucs, which are incremented in the FS or input stream as things happen. There's no per-thread tracking, —its collecting overall stats, rather than trying to add up the cost of a single execution, which is what per-thread stuff would, presumably do. This is lower cost but still permits microbenchmark-style analysis of performance problems against S3a. It doesn't directly let you get results of a job, "34MB of data, 2000 stream aborts, 1998 backward seeks" which are the kind of things I'm curious about. I think adding some duration tracking for the blobstore ops would be good too; I used that in [ Swift ] where it helped show that one of the public endpoints was throttling delete calls, so timing out tests. Again, that points more to the classic metrics stuff, or, even better Coda Hale histograms Maybe, and this would be nice, whatever is implemented here is (a) extensible to support some duration type too, at least in parallel, and (b) could be used as a back end by both Metrics2 and Coda Hale metrics registries. That way the slightly more expensive metric systems would have access to this more raw data.
          Hide
          mingma Ming Ma added a comment -

          This is a great idea. It also addresses an issue at the consumption side. MR iterates through these counters in all FileSystems regardless they are implemented or not, which will increase the number of MR Counters causing unnecessary overhead at MR layer such as rpc, history file, webUI, etc.

          As part of the patch evaluation, it might be useful to try this out with MR end to end. Ideally MR doesn't need to change code each time a new method is added to HDFS ClientProtocol. If so, MR File System Counter needs to be more dynamic; maybe the more descriptive File System Counter string should come from FileSystem, etc. It might help to flush out some of the details.

          Show
          mingma Ming Ma added a comment - This is a great idea. It also addresses an issue at the consumption side. MR iterates through these counters in all FileSystems regardless they are implemented or not, which will increase the number of MR Counters causing unnecessary overhead at MR layer such as rpc, history file, webUI, etc. As part of the patch evaluation, it might be useful to try this out with MR end to end. Ideally MR doesn't need to change code each time a new method is added to HDFS ClientProtocol. If so, MR File System Counter needs to be more dynamic; maybe the more descriptive File System Counter string should come from FileSystem, etc. It might help to flush out some of the details.
          Hide
          liuml07 Mingliang Liu added a comment -

          Hi Colin P. McCabe, I think you proposed an innovative idea for supporting the per-operation stats. I'm willing to change the current design if your idea makes more sense in the long term. I'll prepare a full patch after review your sample code.

          Show
          liuml07 Mingliang Liu added a comment - Hi Colin P. McCabe , I think you proposed an innovative idea for supporting the per-operation stats. I'm willing to change the current design if your idea makes more sense in the long term. I'll prepare a full patch after review your sample code.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s 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 6m 45s trunk passed
          +1 compile 5m 44s trunk passed with JDK v1.8.0_77
          +1 compile 6m 38s trunk passed with JDK v1.7.0_95
          +1 checkstyle 0m 23s trunk passed
          +1 mvnsite 0m 57s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 1m 34s trunk passed
          +1 javadoc 0m 56s trunk passed with JDK v1.8.0_77
          +1 javadoc 1m 3s trunk passed with JDK v1.7.0_95
          +1 mvninstall 0m 41s the patch passed
          +1 compile 5m 53s the patch passed with JDK v1.8.0_77
          +1 javac 5m 53s the patch passed
          +1 compile 6m 42s the patch passed with JDK v1.7.0_95
          +1 javac 6m 42s the patch passed
          -1 checkstyle 0m 23s hadoop-common-project/hadoop-common: patch generated 29 new + 131 unchanged - 0 fixed = 160 total (was 131)
          +1 mvnsite 0m 55s the patch passed
          +1 mvneclipse 0m 14s the patch passed
          -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix.
          -1 findbugs 1m 52s hadoop-common-project/hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 0m 53s 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 37s hadoop-common in the patch failed with JDK v1.8.0_77.
          -1 unit 7m 44s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 24s Patch does not generate ASF License warnings.
          60m 0s



          Reason Tests
          FindBugs module:hadoop-common-project/hadoop-common
            Should org.apache.hadoop.fs.FileSystemStorageStatistics$LongStatisticIterator be a static inner class? At FileSystemStorageStatistics.java:inner class? At FileSystemStorageStatistics.java:[lines 50-74]
          JDK v1.8.0_77 Failed junit tests hadoop.fs.TestFilterFileSystem
            hadoop.fs.TestHarFileSystem
          JDK v1.7.0_95 Failed junit tests hadoop.fs.TestFilterFileSystem
            hadoop.fs.TestHarFileSystem



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12799860/HDFS-10175.006.patch
          JIRA Issue HDFS-10175
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7311e739f2f9 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 / 63ac2db
          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-HDFS-Build/15230/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/15230/artifact/patchprocess/whitespace-eol.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/15230/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15230/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_77.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15230/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15230/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15230/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-HDFS-Build/15230/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15230/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 15s 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 6m 45s trunk passed +1 compile 5m 44s trunk passed with JDK v1.8.0_77 +1 compile 6m 38s trunk passed with JDK v1.7.0_95 +1 checkstyle 0m 23s trunk passed +1 mvnsite 0m 57s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 1m 34s trunk passed +1 javadoc 0m 56s trunk passed with JDK v1.8.0_77 +1 javadoc 1m 3s trunk passed with JDK v1.7.0_95 +1 mvninstall 0m 41s the patch passed +1 compile 5m 53s the patch passed with JDK v1.8.0_77 +1 javac 5m 53s the patch passed +1 compile 6m 42s the patch passed with JDK v1.7.0_95 +1 javac 6m 42s the patch passed -1 checkstyle 0m 23s hadoop-common-project/hadoop-common: patch generated 29 new + 131 unchanged - 0 fixed = 160 total (was 131) +1 mvnsite 0m 55s the patch passed +1 mvneclipse 0m 14s the patch passed -1 whitespace 0m 0s The patch has 2 line(s) that end in whitespace. Use git apply --whitespace=fix. -1 findbugs 1m 52s hadoop-common-project/hadoop-common generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 0m 53s 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 37s hadoop-common in the patch failed with JDK v1.8.0_77. -1 unit 7m 44s hadoop-common in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 24s Patch does not generate ASF License warnings. 60m 0s Reason Tests FindBugs module:hadoop-common-project/hadoop-common   Should org.apache.hadoop.fs.FileSystemStorageStatistics$LongStatisticIterator be a static inner class? At FileSystemStorageStatistics.java:inner class? At FileSystemStorageStatistics.java: [lines 50-74] JDK v1.8.0_77 Failed junit tests hadoop.fs.TestFilterFileSystem   hadoop.fs.TestHarFileSystem JDK v1.7.0_95 Failed junit tests hadoop.fs.TestFilterFileSystem   hadoop.fs.TestHarFileSystem Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12799860/HDFS-10175.006.patch JIRA Issue HDFS-10175 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7311e739f2f9 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 / 63ac2db 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-HDFS-Build/15230/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/15230/artifact/patchprocess/whitespace-eol.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/15230/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/15230/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_77.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15230/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15230/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15230/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-HDFS-Build/15230/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15230/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          cmccabe Colin P. McCabe added a comment -

          I posted patch 006 as an example of what I was thinking about. The idea would be that some FS subclasses like DistributedFileSystem would override FileSystem#getStorageStatistics to return a StorageStatistics object with more details. In the case of DistributedFileSystem (HDFS) these would be mostly about the number of various RPCs that were done. We would simply use volatile longs to store this information.

          Show
          cmccabe Colin P. McCabe added a comment - I posted patch 006 as an example of what I was thinking about. The idea would be that some FS subclasses like DistributedFileSystem would override FileSystem#getStorageStatistics to return a StorageStatistics object with more details. In the case of DistributedFileSystem (HDFS) these would be mostly about the number of various RPCs that were done. We would simply use volatile longs to store this information.
          Hide
          cmccabe Colin P. McCabe added a comment -

          I thought about this a little bit more, and I don't think that FileSystem#Statistics#StatisticsData is the best place to add these new statistics. There are a few reasons.

          Firstly, the statistics that we're interested in are inherently filesystem-specific. For HDFS, we're interested in the number of RPCs to the NameNode-- calls like primitiveCreate, getBytesWithFutureGS, or concat. For something like s3a, we're interested in how many PUT and GET requests we've done to Amazon S3. s3 doesn't even support genstamps or the concat operation. Local filesystems have their own operations which are important.

          Secondly, the thread-local-data mechanism is not really that appropriate for most operations. Thread-local data is a big performance win when reading or writing bytes of data from or to a stream, since most such operations don't involve making an RPC. We have big client-side buffers which mean that most reads and writes can return immediately. In constrast, operations like mkdir, rename, delete, etc. always end up making at least one RPC, since these operations cannot be buffered on the client. In that case, the CPU overhead of doing an atomic increment is negligable. But the overhead of storing all that thread-local data is significant.

          I think what we should do is add an API to the FileSystem and FileContext base classes, which different types of FS can implement as appropriate.

          Show
          cmccabe Colin P. McCabe added a comment - I thought about this a little bit more, and I don't think that FileSystem#Statistics#StatisticsData is the best place to add these new statistics. There are a few reasons. Firstly, the statistics that we're interested in are inherently filesystem-specific. For HDFS, we're interested in the number of RPCs to the NameNode-- calls like primitiveCreate, getBytesWithFutureGS, or concat. For something like s3a, we're interested in how many PUT and GET requests we've done to Amazon S3. s3 doesn't even support genstamps or the concat operation. Local filesystems have their own operations which are important. Secondly, the thread-local-data mechanism is not really that appropriate for most operations. Thread-local data is a big performance win when reading or writing bytes of data from or to a stream, since most such operations don't involve making an RPC. We have big client-side buffers which mean that most reads and writes can return immediately. In constrast, operations like mkdir, rename, delete, etc. always end up making at least one RPC, since these operations cannot be buffered on the client. In that case, the CPU overhead of doing an atomic increment is negligable. But the overhead of storing all that thread-local data is significant. I think what we should do is add an API to the FileSystem and FileContext base classes, which different types of FS can implement as appropriate.
          Hide
          cmccabe Colin P. McCabe added a comment -

          I'll take a look tomorrow, Mingliang Liu. Thanks for working on this.

          Show
          cmccabe Colin P. McCabe added a comment - I'll take a look tomorrow, Mingliang Liu . Thanks for working on this.
          Hide
          liuml07 Mingliang Liu added a comment -

          Hi Colin P. McCabe, would you kindly have a look at the current patch please? Is it OK according to our discussion?

          Show
          liuml07 Mingliang Liu added a comment - Hi Colin P. McCabe , would you kindly have a look at the current patch please? Is it OK according to our discussion?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 19s 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.
          0 mvndep 0m 55s Maven dependency ordering for branch
          +1 mvninstall 10m 21s trunk passed
          +1 compile 12m 24s trunk passed with JDK v1.8.0_77
          +1 compile 10m 36s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 38s trunk passed
          +1 mvnsite 3m 33s trunk passed
          +1 mvneclipse 1m 8s trunk passed
          +1 findbugs 7m 3s trunk passed
          +1 javadoc 3m 48s trunk passed with JDK v1.8.0_77
          +1 javadoc 5m 2s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 24s Maven dependency ordering for patch
          +1 mvninstall 3m 1s the patch passed
          +1 compile 12m 4s the patch passed with JDK v1.8.0_77
          +1 javac 12m 4s the patch passed
          +1 compile 10m 32s the patch passed with JDK v1.7.0_95
          +1 javac 10m 32s the patch passed
          -1 checkstyle 1m 35s root: patch generated 1 new + 211 unchanged - 1 fixed = 212 total (was 212)
          +1 mvnsite 3m 26s the patch passed
          +1 mvneclipse 1m 6s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 7m 57s the patch passed
          +1 javadoc 3m 42s the patch passed with JDK v1.8.0_77
          +1 javadoc 4m 54s the patch passed with JDK v1.7.0_95
          -1 unit 11m 9s hadoop-common in the patch failed with JDK v1.8.0_77.
          +1 unit 1m 31s hadoop-hdfs-client in the patch passed with JDK v1.8.0_77.
          -1 unit 76m 36s hadoop-hdfs in the patch failed with JDK v1.8.0_77.
          -1 unit 10m 59s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 unit 1m 26s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 76m 25s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 59s Patch does not generate ASF License warnings.
          287m 8s



          Reason Tests
          JDK v1.8.0_77 Failed junit tests hadoop.ipc.TestRPCWaitForProxy
            hadoop.net.TestDNS
            hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes
            hadoop.hdfs.server.datanode.TestDirectoryScanner
            hadoop.hdfs.server.namenode.TestEditLog
            hadoop.hdfs.server.namenode.TestDecommissioningStatus
            hadoop.hdfs.server.namenode.TestReconstructStripedBlocks
            hadoop.hdfs.server.namenode.ha.TestEditLogTailer
            hadoop.hdfs.server.datanode.TestDataNodeUUID
            hadoop.hdfs.server.datanode.TestDataNodeMultipleRegistrations
            hadoop.hdfs.shortcircuit.TestShortCircuitLocalRead
            hadoop.hdfs.security.TestDelegationTokenForProxyUser
            hadoop.hdfs.TestFileAppend
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl
          JDK v1.8.0_77 Timed out junit tests org.apache.hadoop.hdfs.TestDFSClientRetries
            org.apache.hadoop.hdfs.server.balancer.TestBalancer
            org.apache.hadoop.hdfs.TestReadStripedFileWithDecoding
            org.apache.hadoop.hdfs.server.datanode.TestDirectoryScanner
          JDK v1.7.0_95 Failed junit tests hadoop.ipc.TestRPCWaitForProxy
            hadoop.hdfs.server.datanode.TestDirectoryScanner
            hadoop.hdfs.shortcircuit.TestShortCircuitLocalRead
            hadoop.hdfs.server.blockmanagement.TestPendingInvalidateBlock
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl
          JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.hdfs.TestParallelShortCircuitLegacyRead
            org.apache.hadoop.hdfs.TestAclsEndToEnd
            org.apache.hadoop.hdfs.TestDFSStorageStateRecovery
            org.apache.hadoop.hdfs.TestFileAppend2



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12799057/HDFS-10175.005.patch
          JIRA Issue HDFS-10175
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7868aa0a2d5d 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 / cab9cba
          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-HDFS-Build/15177/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_77.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_77.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15177/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-HDFS-Build/15177/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15177/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 19s 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. 0 mvndep 0m 55s Maven dependency ordering for branch +1 mvninstall 10m 21s trunk passed +1 compile 12m 24s trunk passed with JDK v1.8.0_77 +1 compile 10m 36s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 38s trunk passed +1 mvnsite 3m 33s trunk passed +1 mvneclipse 1m 8s trunk passed +1 findbugs 7m 3s trunk passed +1 javadoc 3m 48s trunk passed with JDK v1.8.0_77 +1 javadoc 5m 2s trunk passed with JDK v1.7.0_95 0 mvndep 0m 24s Maven dependency ordering for patch +1 mvninstall 3m 1s the patch passed +1 compile 12m 4s the patch passed with JDK v1.8.0_77 +1 javac 12m 4s the patch passed +1 compile 10m 32s the patch passed with JDK v1.7.0_95 +1 javac 10m 32s the patch passed -1 checkstyle 1m 35s root: patch generated 1 new + 211 unchanged - 1 fixed = 212 total (was 212) +1 mvnsite 3m 26s the patch passed +1 mvneclipse 1m 6s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 7m 57s the patch passed +1 javadoc 3m 42s the patch passed with JDK v1.8.0_77 +1 javadoc 4m 54s the patch passed with JDK v1.7.0_95 -1 unit 11m 9s hadoop-common in the patch failed with JDK v1.8.0_77. +1 unit 1m 31s hadoop-hdfs-client in the patch passed with JDK v1.8.0_77. -1 unit 76m 36s hadoop-hdfs in the patch failed with JDK v1.8.0_77. -1 unit 10m 59s hadoop-common in the patch failed with JDK v1.7.0_95. +1 unit 1m 26s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 76m 25s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 59s Patch does not generate ASF License warnings. 287m 8s Reason Tests JDK v1.8.0_77 Failed junit tests hadoop.ipc.TestRPCWaitForProxy   hadoop.net.TestDNS   hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes   hadoop.hdfs.server.datanode.TestDirectoryScanner   hadoop.hdfs.server.namenode.TestEditLog   hadoop.hdfs.server.namenode.TestDecommissioningStatus   hadoop.hdfs.server.namenode.TestReconstructStripedBlocks   hadoop.hdfs.server.namenode.ha.TestEditLogTailer   hadoop.hdfs.server.datanode.TestDataNodeUUID   hadoop.hdfs.server.datanode.TestDataNodeMultipleRegistrations   hadoop.hdfs.shortcircuit.TestShortCircuitLocalRead   hadoop.hdfs.security.TestDelegationTokenForProxyUser   hadoop.hdfs.TestFileAppend   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl JDK v1.8.0_77 Timed out junit tests org.apache.hadoop.hdfs.TestDFSClientRetries   org.apache.hadoop.hdfs.server.balancer.TestBalancer   org.apache.hadoop.hdfs.TestReadStripedFileWithDecoding   org.apache.hadoop.hdfs.server.datanode.TestDirectoryScanner JDK v1.7.0_95 Failed junit tests hadoop.ipc.TestRPCWaitForProxy   hadoop.hdfs.server.datanode.TestDirectoryScanner   hadoop.hdfs.shortcircuit.TestShortCircuitLocalRead   hadoop.hdfs.server.blockmanagement.TestPendingInvalidateBlock   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl JDK v1.7.0_95 Timed out junit tests org.apache.hadoop.hdfs.TestParallelShortCircuitLegacyRead   org.apache.hadoop.hdfs.TestAclsEndToEnd   org.apache.hadoop.hdfs.TestDFSStorageStateRecovery   org.apache.hadoop.hdfs.TestFileAppend2 Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12799057/HDFS-10175.005.patch JIRA Issue HDFS-10175 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7868aa0a2d5d 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 / cab9cba 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-HDFS-Build/15177/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_77.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_77.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15177/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15177/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-HDFS-Build/15177/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15177/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks Colin P. McCabe very much for the discussion.

          1. The v5 patch is to make the detailed statistics optional. As the statistics is shared among file system objects, it's hard to enable/disable it using per-file-system config key. The patch added a new API to Statistics to enable/disable this feature according to the tradeoff between reduced cost and detailed per-op counter. The extra overhead should be avoided if the enum map is not constructed.
          2. I filed HADOOP-13031 to track the discussion and effort of refactoring the code that maintains rack-aware counters. Specially, I also think it's not good to expose the internal composite data structure of distance-aware bytes read. Those use cases that iterate all the distances will call getBytesReadByDistance(int distance) multiple times, which internally will trigger the aggregation among all threads statistics data multiple times. To address this, perhaps they can use the getData() to get all the statistics data at once. I reviewed the current patch iof MAPREDUCE-6660 which employs the bytes-read-by-distance, and found it used the getData() as I expected.
          3. Based on the current FileSystem design, many of which are HDFS specific, we see no better choice than putting them in FileSystem$Statistics, for supporting either distance-aware read counters (HDFS-specific) or per-operation-counters (many of which are HDFS specific). By now, when the detailed statistics are missing (e.g. S3AFileSystem#append()), we treat it as zero. If some operations' statistics are different, they can update the statistics accordingly (e.g. S3AFileSystem#rename) as the counters are populated in concrete file system operations. Another point is that, for existing readOps/writeOps counters, we also have similar scenario (and challenges). Will file follow-up jiras if we have specific cases to handle.
          4. I created a new jira HADOOP-13032 to track the effort of moving the Statistics class out of FileSystem for shorter source code and simpler class structure, thought incompatible change.
          Show
          liuml07 Mingliang Liu added a comment - Thanks Colin P. McCabe very much for the discussion. The v5 patch is to make the detailed statistics optional. As the statistics is shared among file system objects, it's hard to enable/disable it using per-file-system config key. The patch added a new API to Statistics to enable/disable this feature according to the tradeoff between reduced cost and detailed per-op counter. The extra overhead should be avoided if the enum map is not constructed. I filed HADOOP-13031 to track the discussion and effort of refactoring the code that maintains rack-aware counters. Specially, I also think it's not good to expose the internal composite data structure of distance-aware bytes read. Those use cases that iterate all the distances will call getBytesReadByDistance(int distance) multiple times, which internally will trigger the aggregation among all threads statistics data multiple times. To address this, perhaps they can use the getData() to get all the statistics data at once. I reviewed the current patch iof MAPREDUCE-6660 which employs the bytes-read-by-distance, and found it used the getData() as I expected. Based on the current FileSystem design, many of which are HDFS specific, we see no better choice than putting them in FileSystem$Statistics, for supporting either distance-aware read counters (HDFS-specific) or per-operation-counters (many of which are HDFS specific). By now, when the detailed statistics are missing (e.g. S3AFileSystem#append() ), we treat it as zero. If some operations' statistics are different, they can update the statistics accordingly (e.g. S3AFileSystem#rename ) as the counters are populated in concrete file system operations. Another point is that, for existing readOps/writeOps counters, we also have similar scenario (and challenges). Will file follow-up jiras if we have specific cases to handle. I created a new jira HADOOP-13032 to track the effort of moving the Statistics class out of FileSystem for shorter source code and simpler class structure, thought incompatible change.
          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 1 new or modified test files.
          0 mvndep 0m 34s Maven dependency ordering for branch
          +1 mvninstall 6m 48s trunk passed
          +1 compile 6m 0s trunk passed with JDK v1.8.0_77
          +1 compile 6m 45s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 7s trunk passed
          +1 mvnsite 2m 19s trunk passed
          +1 mvneclipse 0m 39s trunk passed
          +1 findbugs 5m 1s trunk passed
          +1 javadoc 2m 21s trunk passed with JDK v1.8.0_77
          +1 javadoc 3m 13s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 27s Maven dependency ordering for patch
          +1 mvninstall 1m 57s the patch passed
          +1 compile 5m 58s the patch passed with JDK v1.8.0_77
          +1 javac 5m 58s the patch passed
          +1 compile 6m 42s the patch passed with JDK v1.7.0_95
          +1 javac 6m 42s the patch passed
          -1 checkstyle 1m 6s root: patch generated 1 new + 211 unchanged - 1 fixed = 212 total (was 212)
          +1 mvnsite 2m 20s the patch passed
          +1 mvneclipse 0m 40s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 5m 54s the patch passed
          +1 javadoc 2m 20s the patch passed with JDK v1.8.0_77
          +1 javadoc 3m 15s the patch passed with JDK v1.7.0_95
          +1 unit 7m 58s hadoop-common in the patch passed with JDK v1.8.0_77.
          +1 unit 0m 52s hadoop-hdfs-client in the patch passed with JDK v1.8.0_77.
          -1 unit 57m 14s hadoop-hdfs in the patch failed with JDK v1.8.0_77.
          +1 unit 7m 45s hadoop-common in the patch passed with JDK v1.7.0_95.
          +1 unit 0m 58s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 54m 6s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          +1 asflicense 0m 25s Patch does not generate ASF License warnings.
          196m 32s



          Reason Tests
          JDK v1.8.0_77 Failed junit tests hadoop.fs.contract.hdfs.TestHDFSContractSeek
            hadoop.hdfs.server.balancer.TestBalancer
            hadoop.hdfs.TestHFlush
            hadoop.hdfs.TestDFSUpgradeFromImage
          JDK v1.7.0_95 Failed junit tests hadoop.fs.contract.hdfs.TestHDFSContractSeek
            hadoop.hdfs.TestHFlush



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:fbe3e86
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12798136/HDFS-10175.004.patch
          JIRA Issue HDFS-10175
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 4e7da1085823 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 / 44bbc50
          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-HDFS-Build/15137/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15137/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_77.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15137/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15137/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15137/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-HDFS-Build/15137/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15137/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 1 new or modified test files. 0 mvndep 0m 34s Maven dependency ordering for branch +1 mvninstall 6m 48s trunk passed +1 compile 6m 0s trunk passed with JDK v1.8.0_77 +1 compile 6m 45s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 7s trunk passed +1 mvnsite 2m 19s trunk passed +1 mvneclipse 0m 39s trunk passed +1 findbugs 5m 1s trunk passed +1 javadoc 2m 21s trunk passed with JDK v1.8.0_77 +1 javadoc 3m 13s trunk passed with JDK v1.7.0_95 0 mvndep 0m 27s Maven dependency ordering for patch +1 mvninstall 1m 57s the patch passed +1 compile 5m 58s the patch passed with JDK v1.8.0_77 +1 javac 5m 58s the patch passed +1 compile 6m 42s the patch passed with JDK v1.7.0_95 +1 javac 6m 42s the patch passed -1 checkstyle 1m 6s root: patch generated 1 new + 211 unchanged - 1 fixed = 212 total (was 212) +1 mvnsite 2m 20s the patch passed +1 mvneclipse 0m 40s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 5m 54s the patch passed +1 javadoc 2m 20s the patch passed with JDK v1.8.0_77 +1 javadoc 3m 15s the patch passed with JDK v1.7.0_95 +1 unit 7m 58s hadoop-common in the patch passed with JDK v1.8.0_77. +1 unit 0m 52s hadoop-hdfs-client in the patch passed with JDK v1.8.0_77. -1 unit 57m 14s hadoop-hdfs in the patch failed with JDK v1.8.0_77. +1 unit 7m 45s hadoop-common in the patch passed with JDK v1.7.0_95. +1 unit 0m 58s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 54m 6s hadoop-hdfs in the patch failed with JDK v1.7.0_95. +1 asflicense 0m 25s Patch does not generate ASF License warnings. 196m 32s Reason Tests JDK v1.8.0_77 Failed junit tests hadoop.fs.contract.hdfs.TestHDFSContractSeek   hadoop.hdfs.server.balancer.TestBalancer   hadoop.hdfs.TestHFlush   hadoop.hdfs.TestDFSUpgradeFromImage JDK v1.7.0_95 Failed junit tests hadoop.fs.contract.hdfs.TestHDFSContractSeek   hadoop.hdfs.TestHFlush Subsystem Report/Notes Docker Image:yetus/hadoop:fbe3e86 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12798136/HDFS-10175.004.patch JIRA Issue HDFS-10175 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4e7da1085823 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 / 44bbc50 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-HDFS-Build/15137/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15137/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_77.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15137/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15137/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_77.txt https://builds.apache.org/job/PreCommit-HDFS-Build/15137/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-HDFS-Build/15137/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15137/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          cmccabe Colin P. McCabe added a comment - - edited

          Thanks, Ming Ma. I agree that if we are going with a map-based approach, we can simplify the implementation of HDFS-9579. This may also save a small amount of space... for example, if FileSystem threads don't have any bytes read at distance 3 or 4, they will no longer need to store space for bytesReadDistanceOfThreeOrFour.

          It also seems like a good idea to make the detailed statistics optional. Then clients could opt-in to paying the overheads, rather than getting the overheads imposed whether they want them or not.

          Mingliang Liu wrote:

          2) Move long getBytesReadByDistance(int distance) from Statistics to StatisticsData. If the user is getting bytes read for all distances, she can call getData() and then iterate the map/array, in which case the getData() will be called only once. For cases of 1K client threads, this may save the effort of aggregation. Colin Patrick McCabe may have different comments about this?

          Hmm. I'm not sure I see much of a benefit to this. Users should not be iterating over internal data structures. It exposes implementation details that we don't want to expose. I also don't see how it's more efficient, since you have to iterate over it in either case.

          For newly supported APIs, adding an entry in the map and one line of increment in the new method will make the magic done. From the point of file system APIs, its public methods are not evolving rapidly.

          I agree that adding new statistics is relatively easy with the current patch. My comment was more that currently, many of these statistics are HDFS-specific. Since MapReduce needs to work with alternate filesystems, it will need to handle the case where these detailed statistics are missing or slightly different.

          Another dimension will be needed for cross-DC analysis, while based on the current use case, I don't think this dimension is heavily needed. One point is that, all the same kind of file system is sharing the statistic data among threads, regardless of the remote/local HDFS clusters.

          (revised earlier comment about per-class stats) Yes, it does appear that stats are per-class.

          Show
          cmccabe Colin P. McCabe added a comment - - edited Thanks, Ming Ma . I agree that if we are going with a map-based approach, we can simplify the implementation of HDFS-9579 . This may also save a small amount of space... for example, if FileSystem threads don't have any bytes read at distance 3 or 4, they will no longer need to store space for bytesReadDistanceOfThreeOrFour . It also seems like a good idea to make the detailed statistics optional. Then clients could opt-in to paying the overheads, rather than getting the overheads imposed whether they want them or not. Mingliang Liu wrote: 2) Move long getBytesReadByDistance(int distance) from Statistics to StatisticsData. If the user is getting bytes read for all distances, she can call getData() and then iterate the map/array, in which case the getData() will be called only once. For cases of 1K client threads, this may save the effort of aggregation. Colin Patrick McCabe may have different comments about this? Hmm. I'm not sure I see much of a benefit to this. Users should not be iterating over internal data structures. It exposes implementation details that we don't want to expose. I also don't see how it's more efficient, since you have to iterate over it in either case. For newly supported APIs, adding an entry in the map and one line of increment in the new method will make the magic done. From the point of file system APIs, its public methods are not evolving rapidly. I agree that adding new statistics is relatively easy with the current patch. My comment was more that currently, many of these statistics are HDFS-specific. Since MapReduce needs to work with alternate filesystems, it will need to handle the case where these detailed statistics are missing or slightly different. Another dimension will be needed for cross-DC analysis, while based on the current use case, I don't think this dimension is heavily needed. One point is that, all the same kind of file system is sharing the statistic data among threads, regardless of the remote/local HDFS clusters. (revised earlier comment about per-class stats) Yes, it does appear that stats are per-class.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thank you very much Ming Ma for the comments.

          Yes NN also has audit log which tracks DFS operations. The missing DFSClient name makes it less useful for the user. Your point about the motivation and map vs. inline log implementation of HDFS-9579 made a lot of sense to me. I see no performance benefit from map approach. I was wondering whether using a composite (e.g. enum map, array) data structure to manage the distance->bytesRead mapping makes the code simpler.
          0) StatisticsData should be a bit shorter by delegating the operations to the composite data structure.
          1) The incrementBytesReadByDistance(int distance, long newBytes) and getBytesReadByDistance(int distance) which switch-case all hard-code variables, may be simplified as we can set/get the bytesRead by distance directly from map/array.
          2) Move long getBytesReadByDistance(int distance) from Statistics to StatisticsData. If the user is getting bytes read for all distances, she can call getData() and then iterate the map/array, in which case the getData() will be called only once. For cases of 1K client threads, this may save the effort of aggregation.
          Colin P. McCabe may have different comments about this?

          For newly supported APIs, adding an entry in the map and one line of increment in the new method will make the magic done. From the point of file system APIs, its public methods are not evolving rapidly. Another dimension will be needed for cross-DC analysis, while based on the current use case, I don't think this dimension is heavily needed. One point is that, all the same kind of file system is sharing the statistic data among threads, regardless of the remote/local HDFS clusters.

          Jitendra Nath Pandey also suggested to make this feature optional/configurable. I found it hard largely because different file system object has individual configurations. As they share the statistic data by FS class type (scheme), it will be confusing if one FS disables this feature and another FS enables this features. Is there any easy approach to handling this case?

          p.s. the v4 patch is to address the HAS_NEXT/LIST_STATUS.

          Show
          liuml07 Mingliang Liu added a comment - Thank you very much Ming Ma for the comments. Yes NN also has audit log which tracks DFS operations. The missing DFSClient name makes it less useful for the user. Your point about the motivation and map vs. inline log implementation of HDFS-9579 made a lot of sense to me. I see no performance benefit from map approach. I was wondering whether using a composite (e.g. enum map, array) data structure to manage the distance->bytesRead mapping makes the code simpler. 0) StatisticsData should be a bit shorter by delegating the operations to the composite data structure. 1) The incrementBytesReadByDistance(int distance, long newBytes) and getBytesReadByDistance(int distance) which switch-case all hard-code variables, may be simplified as we can set/get the bytesRead by distance directly from map/array. 2) Move long getBytesReadByDistance(int distance) from Statistics to StatisticsData . If the user is getting bytes read for all distances, she can call getData() and then iterate the map/array, in which case the getData() will be called only once. For cases of 1K client threads, this may save the effort of aggregation. Colin P. McCabe may have different comments about this? For newly supported APIs, adding an entry in the map and one line of increment in the new method will make the magic done. From the point of file system APIs, its public methods are not evolving rapidly. Another dimension will be needed for cross-DC analysis, while based on the current use case, I don't think this dimension is heavily needed. One point is that, all the same kind of file system is sharing the statistic data among threads, regardless of the remote/local HDFS clusters. Jitendra Nath Pandey also suggested to make this feature optional/configurable. I found it hard largely because different file system object has individual configurations. As they share the statistic data by FS class type (scheme), it will be confusing if one FS disables this feature and another FS enables this features. Is there any easy approach to handling this case? p.s. the v4 patch is to address the HAS_NEXT / LIST_STATUS .
          Hide
          mingma Ming Ma added a comment -

          Interesting work.

          • Seems like a useful feature to provide per-job counters for this. Note that NN also has the data, except nntop or audit log don't track them on per DFSClient client name basis. For the HDFS-9579 scenario, besides the per-job counters functionality, another motivation is the extra work required to track and aggregate on DN side (something we still want to experiment to track hot block, or remote read or write for all HDFS apps, not just MR).
          • Question about using inline long or map object. The original patch in HDFS-9579 used map as I tried to make it more dynamic in two aspects; no need to hard code network distance or memory allocation for FileSystems that don't generate such data. But the small extra memory for static longs and mostly static network distance definition make map approach less interesting.
          • Growth of the metrics. It seems each time we add a RPC method, we need to add another entry. Also if someone wants to track cross-DC mkdir and intra-DC mkdir, then you added another dimension to it.
          • Making it optional is a good idea.
          Show
          mingma Ming Ma added a comment - Interesting work. Seems like a useful feature to provide per-job counters for this. Note that NN also has the data, except nntop or audit log don't track them on per DFSClient client name basis. For the HDFS-9579 scenario, besides the per-job counters functionality, another motivation is the extra work required to track and aggregate on DN side (something we still want to experiment to track hot block, or remote read or write for all HDFS apps, not just MR). Question about using inline long or map object. The original patch in HDFS-9579 used map as I tried to make it more dynamic in two aspects; no need to hard code network distance or memory allocation for FileSystems that don't generate such data. But the small extra memory for static longs and mostly static network distance definition make map approach less interesting. Growth of the metrics. It seems each time we add a RPC method, we need to add another entry. Also if someone wants to track cross-DC mkdir and intra-DC mkdir, then you added another dimension to it. Making it optional is a good idea.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks for your test to quantify the overhead of the per-op stats map. The data you got is similar to our offline analysis. As to the goal that the ~1.5K overhead per FileSystem/thread is trading for, I think the basic idea is to provide file system operation summary in a fine-grained granularity. Before this, we have FS operations counters as read ops, write ops, and largeReadOps etc, which are not very useful for offline load analysis. One simple use case is: one directory is polluted by 1K small files because a job forgets to delete the temporary files after using them. The create/delete counters will help users/admins locate the bad job very easily. WriteOps is not very indicative as it counts many other operations. I believe Hitesh Shah can show us more examples if you find his comment above not quite clear.

          As the statistics is used by other file systems (e.g. S3A) besides HDFS, the per-operation counters can be supported by those file systems. FileSystem itself has several high-level operations that are not supported by all concrete file systems, in which case a zero counter value for an unsupported operation seems OK. Maybe Jitendra Nath Pandey has more comments about this problem.

          Yes the HAS_NEXT is not very a metric that is interesting to users. I'll update the patch for iterative listStatus. This is a very good catch.

          In the very early stage of the HDFS-9579, it used a map (perhaps because it's more straightforward). I think the idea of moving hard-coded statistics longs into maps in this case is good. I'll file new jira to address this. Ping Ming Ma for more input.

          By the way, I think it's good to move the Statistics class out of FileSystem. However, it's incompatible as the usage of "import" should be updated in upper applications.

          Show
          liuml07 Mingliang Liu added a comment - Thanks for your test to quantify the overhead of the per-op stats map. The data you got is similar to our offline analysis. As to the goal that the ~1.5K overhead per FileSystem/thread is trading for, I think the basic idea is to provide file system operation summary in a fine-grained granularity. Before this, we have FS operations counters as read ops, write ops, and largeReadOps etc, which are not very useful for offline load analysis. One simple use case is: one directory is polluted by 1K small files because a job forgets to delete the temporary files after using them. The create/delete counters will help users/admins locate the bad job very easily. WriteOps is not very indicative as it counts many other operations. I believe Hitesh Shah can show us more examples if you find his comment above not quite clear. As the statistics is used by other file systems (e.g. S3A) besides HDFS, the per-operation counters can be supported by those file systems. FileSystem itself has several high-level operations that are not supported by all concrete file systems, in which case a zero counter value for an unsupported operation seems OK. Maybe Jitendra Nath Pandey has more comments about this problem. Yes the HAS_NEXT is not very a metric that is interesting to users. I'll update the patch for iterative listStatus . This is a very good catch. In the very early stage of the HDFS-9579 , it used a map (perhaps because it's more straightforward). I think the idea of moving hard-coded statistics longs into maps in this case is good. I'll file new jira to address this. Ping Ming Ma for more input. By the way, I think it's good to move the Statistics class out of FileSystem . However, it's incompatible as the usage of "import" should be updated in upper applications.
          Hide
          cmccabe Colin P. McCabe added a comment -

          I wrote a test named TestStatisticsOverhead to quantify the overheads more precisely. On my machine, the difference between running with this patch and running without it is about 3MB, as quantified by getHeapMemoryUsage. This is with 2000 threads. Since this is a per-FS overhead, some applications that use 3 or 4 filesystem objects may experience 3x or 4x that memory overhead.

          I agree that that this level of overhead is not a deal-breaker, but I would like to understand what we are getting in exchange. Can you be a little bit clearer about the goals for this? Right now it seems to support only HDFS. And many of the operations are things that only HDFS will ever support, such as getBytesWithFutureGenerationStamps. Some operations are not used at all, such as COPY_FROM_LOCAL_FILE. It's a mishmash of high-level operations such as copy_from_local_file, which involve multiple FSes, and very low-level things like whether or not we called mkdirs or primitiveMkdir. Are you planning to add this statistics support to other filesystems as well?

          HAS_NEXT does not seem like a metric that users would be interested in. If we do another listStatus RPC, we should increment listStatus.

          If we are going to have these maps, we should move the existing hard-coded statistics longs into the maps for simplicity's sake-- or at least things like bytesReadDistanceOfOneOrTwo and bytesReadDistanceOfThreeOrFour.

          Show
          cmccabe Colin P. McCabe added a comment - I wrote a test named TestStatisticsOverhead to quantify the overheads more precisely. On my machine, the difference between running with this patch and running without it is about 3MB, as quantified by getHeapMemoryUsage . This is with 2000 threads. Since this is a per-FS overhead, some applications that use 3 or 4 filesystem objects may experience 3x or 4x that memory overhead. I agree that that this level of overhead is not a deal-breaker, but I would like to understand what we are getting in exchange. Can you be a little bit clearer about the goals for this? Right now it seems to support only HDFS. And many of the operations are things that only HDFS will ever support, such as getBytesWithFutureGenerationStamps . Some operations are not used at all, such as COPY_FROM_LOCAL_FILE . It's a mishmash of high-level operations such as copy_from_local_file, which involve multiple FSes, and very low-level things like whether or not we called mkdirs or primitiveMkdir . Are you planning to add this statistics support to other filesystems as well? HAS_NEXT does not seem like a metric that users would be interested in. If we do another listStatus RPC, we should increment listStatus . If we are going to have these maps, we should move the existing hard-coded statistics longs into the maps for simplicity's sake-- or at least things like bytesReadDistanceOfOneOrTwo and bytesReadDistanceOfThreeOrFour.
          Hide
          jnp Jitendra Nath Pandey added a comment -

          Colin P. McCabe, Are you ok to withdraw your "-1" because Hitesh Shah has explained the use case and the memory overhead seems minimal?

          One more option is to add a configuration parameter at the client side so that per-operation statistics can be disabled if client doesn't want to track them. Mingliang Liu, I believe it should be easy to add a configuration to disable the per-op stats?

          Show
          jnp Jitendra Nath Pandey added a comment - Colin P. McCabe , Are you ok to withdraw your "-1" because Hitesh Shah has explained the use case and the memory overhead seems minimal? One more option is to add a configuration parameter at the client side so that per-operation statistics can be disabled if client doesn't want to track them. Mingliang Liu , I believe it should be easy to add a configuration to disable the per-op stats?
          Hide
          jnp Jitendra Nath Pandey added a comment -

          Each entry in the map should be around 24 bytes (8 bytes for key enum pointer, 8 bytes for value object pointer and 8 bytes for the long counter). The EnumMap uses arrays unlike a hashmap, and enum keys would be singleton. Therefore overheads are negligible.
          For 48 entries it is 1.1KB per FS, and even for 1000 threads it is only 1.1MB. Triple that and its 3.3 MB. For java processes that create 1000 threads this is trivial amount of memory.

          Show
          jnp Jitendra Nath Pandey added a comment - Each entry in the map should be around 24 bytes (8 bytes for key enum pointer, 8 bytes for value object pointer and 8 bytes for the long counter). The EnumMap uses arrays unlike a hashmap, and enum keys would be singleton. Therefore overheads are negligible. For 48 entries it is 1.1KB per FS, and even for 1000 threads it is only 1.1MB. Triple that and its 3.3 MB. For java processes that create 1000 threads this is trivial amount of memory.
          Hide
          cmccabe Colin P. McCabe added a comment -

          It's not a map with 48 entries, it's a map with 48 entries per thread per FS. So if you have 1000 threads, that's 48,000 entries. Double or triple that if you have more than one FS (for example, if you have a local filesystem plus an HDFS filesystem).

          Show
          cmccabe Colin P. McCabe added a comment - It's not a map with 48 entries, it's a map with 48 entries per thread per FS. So if you have 1000 threads, that's 48,000 entries. Double or triple that if you have more than one FS (for example, if you have a local filesystem plus an HDFS filesystem).
          Hide
          hitesh Hitesh Shah added a comment -

          Colin P. McCabe For cluster admins who keep tabs on Hive queries or Pig scripts to see what kind of ops they are doing are using the job counters today. Unfortunately, the current job counters - read ops, write ops. etc are quite old and obsolete and are very high level and cannot be used to find bad jobs which say create 1000s of files ( for partitions, etc ). The approach Jitendra and Minglian seem to be proposing seems to integrate well with Mapreduce, etc where counters could be created to map to the filesystem stats. Additionally, these counters would be available for historical analysis as needed via the job history. This is something which would likely be needed for all jobs.

          To some extent I do agree that all the 50-odd metrics being tracked may not be useful for all use-cases. However, enabling this via htrace and turning on htrace for all jobs is also not an option as it does not allow any selective tracking of only certain info. Htrace being enabled for a certain job means too indepth profiling which is not really needed unless necessary for very specific profiling. Do you have any suggestions on the app layers above HDFS can obtain more in-depth stats but at the same time have all of these stats be accessible for all jobs?

          Show
          hitesh Hitesh Shah added a comment - Colin P. McCabe For cluster admins who keep tabs on Hive queries or Pig scripts to see what kind of ops they are doing are using the job counters today. Unfortunately, the current job counters - read ops, write ops. etc are quite old and obsolete and are very high level and cannot be used to find bad jobs which say create 1000s of files ( for partitions, etc ). The approach Jitendra and Minglian seem to be proposing seems to integrate well with Mapreduce, etc where counters could be created to map to the filesystem stats. Additionally, these counters would be available for historical analysis as needed via the job history. This is something which would likely be needed for all jobs. To some extent I do agree that all the 50-odd metrics being tracked may not be useful for all use-cases. However, enabling this via htrace and turning on htrace for all jobs is also not an option as it does not allow any selective tracking of only certain info. Htrace being enabled for a certain job means too indepth profiling which is not really needed unless necessary for very specific profiling. Do you have any suggestions on the app layers above HDFS can obtain more in-depth stats but at the same time have all of these stats be accessible for all jobs?
          Hide
          jnp Jitendra Nath Pandey added a comment -

          This proposal is very simple and lightweight to collect statistics for various file system operations, as compared to HTrace. HTrace is significantly more complicated to setup and operate. The counters in mapreduce have been in use for a long time and are very useful to gather job level information, which is critical to analyze job behaviors.
          The proposed map has only 48 entries. Being an enum-map it will be significantly optimized. I think this is a very simple patch and a good low hanging fruit for more detailed analysis of job behaviors.

          Show
          jnp Jitendra Nath Pandey added a comment - This proposal is very simple and lightweight to collect statistics for various file system operations, as compared to HTrace. HTrace is significantly more complicated to setup and operate. The counters in mapreduce have been in use for a long time and are very useful to gather job level information, which is critical to analyze job behaviors. The proposed map has only 48 entries. Being an enum-map it will be significantly optimized. I think this is a very simple patch and a good low hanging fruit for more detailed analysis of job behaviors.
          Hide
          cmccabe Colin P. McCabe added a comment -

          I'm still -1 on this change. v3 creates a very large map per FileSystem per thread. For an application with lots of threads, this overhead is unacceptable. HTrace seems like a better way to get this information. I don't see a clear use-case for this.

          Show
          cmccabe Colin P. McCabe added a comment - I'm still -1 on this change. v3 creates a very large map per FileSystem per thread. For an application with lots of threads, this overhead is unacceptable. HTrace seems like a better way to get this information. I don't see a clear use-case for this.
          Hide
          jnp Jitendra Nath Pandey added a comment -

          Colin P. McCabe, and Andrew Wang the latest patch addresses the concerns about synchronization. Do you think it is ok to commit now?

          Show
          jnp Jitendra Nath Pandey added a comment - Colin P. McCabe , and Andrew Wang the latest patch addresses the concerns about synchronization. Do you think it is ok to commit now?
          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 1 new or modified test files.
          0 mvndep 0m 14s Maven dependency ordering for branch
          +1 mvninstall 6m 46s trunk passed
          +1 compile 6m 27s trunk passed with JDK v1.8.0_74
          +1 compile 7m 21s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 6s trunk passed
          +1 mvnsite 2m 35s trunk passed
          +1 mvneclipse 0m 41s trunk passed
          +1 findbugs 5m 36s trunk passed
          +1 javadoc 2m 31s trunk passed with JDK v1.8.0_74
          +1 javadoc 3m 21s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 15s Maven dependency ordering for patch
          +1 mvninstall 2m 1s the patch passed
          +1 compile 6m 16s the patch passed with JDK v1.8.0_74
          +1 javac 6m 16s the patch passed
          +1 compile 7m 3s the patch passed with JDK v1.7.0_95
          +1 javac 7m 3s the patch passed
          -1 checkstyle 1m 7s root: patch generated 1 new + 211 unchanged - 1 fixed = 212 total (was 212)
          +1 mvnsite 2m 27s the patch passed
          +1 mvneclipse 0m 39s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 6m 1s the patch passed
          +1 javadoc 2m 21s the patch passed with JDK v1.8.0_74
          +1 javadoc 3m 17s the patch passed with JDK v1.7.0_95
          -1 unit 7m 1s hadoop-common in the patch failed with JDK v1.8.0_74.
          +1 unit 0m 52s hadoop-hdfs-client in the patch passed with JDK v1.8.0_74.
          +1 unit 58m 52s hadoop-hdfs in the patch passed with JDK v1.8.0_74.
          -1 unit 7m 16s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 unit 1m 0s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 55m 38s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          -1 asflicense 0m 26s Patch generated 2 ASF License warnings.
          201m 1s



          Reason Tests
          JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker
          JDK v1.7.0_95 Failed junit tests hadoop.security.ssl.TestReloadingX509TrustManager
            hadoop.hdfs.TestHFlush
            hadoop.hdfs.server.namenode.TestDecommissioningStatus
          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/12795359/HDFS-10175.003.patch
          JIRA Issue HDFS-10175
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e423bb48277a 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 / 2c268cc
          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
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14937/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-HDFS-Build/14937/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14937/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 1 new or modified test files. 0 mvndep 0m 14s Maven dependency ordering for branch +1 mvninstall 6m 46s trunk passed +1 compile 6m 27s trunk passed with JDK v1.8.0_74 +1 compile 7m 21s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 6s trunk passed +1 mvnsite 2m 35s trunk passed +1 mvneclipse 0m 41s trunk passed +1 findbugs 5m 36s trunk passed +1 javadoc 2m 31s trunk passed with JDK v1.8.0_74 +1 javadoc 3m 21s trunk passed with JDK v1.7.0_95 0 mvndep 0m 15s Maven dependency ordering for patch +1 mvninstall 2m 1s the patch passed +1 compile 6m 16s the patch passed with JDK v1.8.0_74 +1 javac 6m 16s the patch passed +1 compile 7m 3s the patch passed with JDK v1.7.0_95 +1 javac 7m 3s the patch passed -1 checkstyle 1m 7s root: patch generated 1 new + 211 unchanged - 1 fixed = 212 total (was 212) +1 mvnsite 2m 27s the patch passed +1 mvneclipse 0m 39s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 6m 1s the patch passed +1 javadoc 2m 21s the patch passed with JDK v1.8.0_74 +1 javadoc 3m 17s the patch passed with JDK v1.7.0_95 -1 unit 7m 1s hadoop-common in the patch failed with JDK v1.8.0_74. +1 unit 0m 52s hadoop-hdfs-client in the patch passed with JDK v1.8.0_74. +1 unit 58m 52s hadoop-hdfs in the patch passed with JDK v1.8.0_74. -1 unit 7m 16s hadoop-common in the patch failed with JDK v1.7.0_95. +1 unit 1m 0s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 55m 38s hadoop-hdfs in the patch failed with JDK v1.7.0_95. -1 asflicense 0m 26s Patch generated 2 ASF License warnings. 201m 1s Reason Tests JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker JDK v1.7.0_95 Failed junit tests hadoop.security.ssl.TestReloadingX509TrustManager   hadoop.hdfs.TestHFlush   hadoop.hdfs.server.namenode.TestDecommissioningStatus 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/12795359/HDFS-10175.003.patch JIRA Issue HDFS-10175 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e423bb48277a 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 / 2c268cc 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 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14937/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-HDFS-Build/14937/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/14937/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14937/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks Colin P. McCabe for providing comments.

          I agree with you about the synchronization overhead may be a concern for the client side. The v3 patch is to address this following the convention in the current statistics implementation.

          As the description explained, the operation-specific counters can be used for analyzing the load imposed by a particular job on HDFS. Adding the per-operation counter in Statistics object is the most straightforward and simple approach for collecting and exposing this information.

          Show
          liuml07 Mingliang Liu added a comment - Thanks Colin P. McCabe for providing comments. I agree with you about the synchronization overhead may be a concern for the client side. The v3 patch is to address this following the convention in the current statistics implementation. As the description explained, the operation-specific counters can be used for analyzing the load imposed by a particular job on HDFS. Adding the per-operation counter in Statistics object is the most straightforward and simple approach for collecting and exposing this information.
          Hide
          cmccabe Colin P. McCabe added a comment -

          -1. As Andrew Wang mentioned, this change imposes heavy synchronization overheads on everyone. It would be better to gather this sort of information through HTrace so that it could be turned on only for applications being traced.

          Show
          cmccabe Colin P. McCabe added a comment - -1. As Andrew Wang mentioned, this change imposes heavy synchronization overheads on everyone. It would be better to gather this sort of information through HTrace so that it could be turned on only for applications being traced.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 1s The patch appears to include 1 new or modified test files.
          0 mvndep 0m 32s Maven dependency ordering for branch
          +1 mvninstall 6m 44s trunk passed
          +1 compile 6m 5s trunk passed with JDK v1.8.0_74
          +1 compile 6m 46s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 7s trunk passed
          +1 mvnsite 2m 21s trunk passed
          +1 mvneclipse 0m 41s trunk passed
          +1 findbugs 5m 5s trunk passed
          +1 javadoc 2m 22s trunk passed with JDK v1.8.0_74
          +1 javadoc 3m 15s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 15s Maven dependency ordering for patch
          +1 mvninstall 2m 2s the patch passed
          +1 compile 5m 53s the patch passed with JDK v1.8.0_74
          +1 javac 5m 53s the patch passed
          +1 compile 6m 51s the patch passed with JDK v1.7.0_95
          +1 javac 6m 51s the patch passed
          +1 checkstyle 1m 6s root: patch generated 0 new + 211 unchanged - 1 fixed = 211 total (was 212)
          +1 mvnsite 2m 20s the patch passed
          +1 mvneclipse 0m 41s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 5m 46s the patch passed
          +1 javadoc 2m 29s the patch passed with JDK v1.8.0_74
          +1 javadoc 3m 20s the patch passed with JDK v1.7.0_95
          -1 unit 6m 47s hadoop-common in the patch failed with JDK v1.8.0_74.
          +1 unit 0m 52s hadoop-hdfs-client in the patch passed with JDK v1.8.0_74.
          -1 unit 69m 17s hadoop-hdfs in the patch failed with JDK v1.8.0_74.
          -1 unit 7m 6s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 unit 1m 1s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 70m 9s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          -1 asflicense 0m 26s Patch generated 2 ASF License warnings.
          223m 10s



          Reason Tests
          JDK v1.8.0_74 Failed junit tests hadoop.hdfs.TestHFlush
            hadoop.hdfs.server.datanode.TestDataNodeHotSwapVolumes
            hadoop.hdfs.server.namenode.ha.TestInitializeSharedEdits
          JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker
          JDK v1.7.0_95 Failed junit tests hadoop.hdfs.TestHFlush
            hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot
            hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl
            hadoop.hdfs.TestRollingUpgrade
          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/12795132/HDFS-10175.002.patch
          JIRA Issue HDFS-10175
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 99b9034ee428 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 / 19b645c
          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
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14916/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-HDFS-Build/14916/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14916/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 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 1s The patch appears to include 1 new or modified test files. 0 mvndep 0m 32s Maven dependency ordering for branch +1 mvninstall 6m 44s trunk passed +1 compile 6m 5s trunk passed with JDK v1.8.0_74 +1 compile 6m 46s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 7s trunk passed +1 mvnsite 2m 21s trunk passed +1 mvneclipse 0m 41s trunk passed +1 findbugs 5m 5s trunk passed +1 javadoc 2m 22s trunk passed with JDK v1.8.0_74 +1 javadoc 3m 15s trunk passed with JDK v1.7.0_95 0 mvndep 0m 15s Maven dependency ordering for patch +1 mvninstall 2m 2s the patch passed +1 compile 5m 53s the patch passed with JDK v1.8.0_74 +1 javac 5m 53s the patch passed +1 compile 6m 51s the patch passed with JDK v1.7.0_95 +1 javac 6m 51s the patch passed +1 checkstyle 1m 6s root: patch generated 0 new + 211 unchanged - 1 fixed = 211 total (was 212) +1 mvnsite 2m 20s the patch passed +1 mvneclipse 0m 41s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 5m 46s the patch passed +1 javadoc 2m 29s the patch passed with JDK v1.8.0_74 +1 javadoc 3m 20s the patch passed with JDK v1.7.0_95 -1 unit 6m 47s hadoop-common in the patch failed with JDK v1.8.0_74. +1 unit 0m 52s hadoop-hdfs-client in the patch passed with JDK v1.8.0_74. -1 unit 69m 17s hadoop-hdfs in the patch failed with JDK v1.8.0_74. -1 unit 7m 6s hadoop-common in the patch failed with JDK v1.7.0_95. +1 unit 1m 1s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 70m 9s hadoop-hdfs in the patch failed with JDK v1.7.0_95. -1 asflicense 0m 26s Patch generated 2 ASF License warnings. 223m 10s Reason Tests JDK v1.8.0_74 Failed junit tests hadoop.hdfs.TestHFlush   hadoop.hdfs.server.datanode.TestDataNodeHotSwapVolumes   hadoop.hdfs.server.namenode.ha.TestInitializeSharedEdits JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker JDK v1.7.0_95 Failed junit tests hadoop.hdfs.TestHFlush   hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot   hadoop.hdfs.server.datanode.fsdataset.impl.TestFsDatasetImpl   hadoop.hdfs.TestRollingUpgrade 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/12795132/HDFS-10175.002.patch JIRA Issue HDFS-10175 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 99b9034ee428 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 / 19b645c 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 unit https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14916/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-HDFS-Build/14916/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/14916/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14916/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Hi Colin P. McCabe, would you kindly review the patch? I'm not sure I got the point of using thread-local variable to improve read/write performance. Thanks!

          Show
          liuml07 Mingliang Liu added a comment - Hi Colin P. McCabe , would you kindly review the patch? I'm not sure I got the point of using thread-local variable to improve read/write performance. Thanks!
          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 1 new or modified test files.
          0 mvndep 0m 41s Maven dependency ordering for branch
          +1 mvninstall 6m 35s trunk passed
          +1 compile 5m 56s trunk passed with JDK v1.8.0_74
          +1 compile 6m 41s trunk passed with JDK v1.7.0_95
          +1 checkstyle 1m 7s trunk passed
          +1 mvnsite 2m 21s trunk passed
          +1 mvneclipse 0m 40s trunk passed
          +1 findbugs 5m 9s trunk passed
          +1 javadoc 2m 20s trunk passed with JDK v1.8.0_74
          +1 javadoc 3m 17s trunk passed with JDK v1.7.0_95
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 1m 59s the patch passed
          +1 compile 5m 47s the patch passed with JDK v1.8.0_74
          +1 javac 5m 47s the patch passed
          +1 compile 6m 40s the patch passed with JDK v1.7.0_95
          +1 javac 6m 40s the patch passed
          -1 checkstyle 1m 5s root: patch generated 2 new + 211 unchanged - 1 fixed = 213 total (was 212)
          +1 mvnsite 2m 19s the patch passed
          +1 mvneclipse 0m 39s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 5m 53s the patch passed
          +1 javadoc 2m 18s the patch passed with JDK v1.8.0_74
          +1 javadoc 3m 14s the patch passed with JDK v1.7.0_95
          -1 unit 6m 33s hadoop-common in the patch failed with JDK v1.8.0_74.
          +1 unit 0m 49s hadoop-hdfs-client in the patch passed with JDK v1.8.0_74.
          -1 unit 56m 31s hadoop-hdfs in the patch failed with JDK v1.8.0_74.
          -1 unit 7m 9s hadoop-common in the patch failed with JDK v1.7.0_95.
          +1 unit 0m 59s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95.
          -1 unit 59m 50s hadoop-hdfs in the patch failed with JDK v1.7.0_95.
          -1 asflicense 0m 28s Patch generated 2 ASF License warnings.
          199m 2s



          Reason Tests
          JDK v1.8.0_74 Failed junit tests hadoop.net.TestDNS
            hadoop.hdfs.TestDFSUpgradeFromImage
            hadoop.hdfs.TestErasureCodeBenchmarkThroughput
          JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker
          JDK v1.7.0_95 Failed junit tests hadoop.net.TestDNS
            hadoop.hdfs.server.datanode.TestFsDatasetCache
            hadoop.hdfs.server.balancer.TestBalancer
          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/12794633/HDFS-10175.001.patch
          JIRA Issue HDFS-10175
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c9a982eed6ac 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 / e7ed05e
          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
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14886/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-HDFS-Build/14886/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14886/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 1 new or modified test files. 0 mvndep 0m 41s Maven dependency ordering for branch +1 mvninstall 6m 35s trunk passed +1 compile 5m 56s trunk passed with JDK v1.8.0_74 +1 compile 6m 41s trunk passed with JDK v1.7.0_95 +1 checkstyle 1m 7s trunk passed +1 mvnsite 2m 21s trunk passed +1 mvneclipse 0m 40s trunk passed +1 findbugs 5m 9s trunk passed +1 javadoc 2m 20s trunk passed with JDK v1.8.0_74 +1 javadoc 3m 17s trunk passed with JDK v1.7.0_95 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 59s the patch passed +1 compile 5m 47s the patch passed with JDK v1.8.0_74 +1 javac 5m 47s the patch passed +1 compile 6m 40s the patch passed with JDK v1.7.0_95 +1 javac 6m 40s the patch passed -1 checkstyle 1m 5s root: patch generated 2 new + 211 unchanged - 1 fixed = 213 total (was 212) +1 mvnsite 2m 19s the patch passed +1 mvneclipse 0m 39s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 5m 53s the patch passed +1 javadoc 2m 18s the patch passed with JDK v1.8.0_74 +1 javadoc 3m 14s the patch passed with JDK v1.7.0_95 -1 unit 6m 33s hadoop-common in the patch failed with JDK v1.8.0_74. +1 unit 0m 49s hadoop-hdfs-client in the patch passed with JDK v1.8.0_74. -1 unit 56m 31s hadoop-hdfs in the patch failed with JDK v1.8.0_74. -1 unit 7m 9s hadoop-common in the patch failed with JDK v1.7.0_95. +1 unit 0m 59s hadoop-hdfs-client in the patch passed with JDK v1.7.0_95. -1 unit 59m 50s hadoop-hdfs in the patch failed with JDK v1.7.0_95. -1 asflicense 0m 28s Patch generated 2 ASF License warnings. 199m 2s Reason Tests JDK v1.8.0_74 Failed junit tests hadoop.net.TestDNS   hadoop.hdfs.TestDFSUpgradeFromImage   hadoop.hdfs.TestErasureCodeBenchmarkThroughput JDK v1.8.0_74 Timed out junit tests org.apache.hadoop.util.TestNativeLibraryChecker JDK v1.7.0_95 Failed junit tests hadoop.net.TestDNS   hadoop.hdfs.server.datanode.TestFsDatasetCache   hadoop.hdfs.server.balancer.TestBalancer 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/12794633/HDFS-10175.001.patch JIRA Issue HDFS-10175 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c9a982eed6ac 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 / e7ed05e 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 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.7.0_95.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs-jdk1.8.0_74.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common-jdk1.7.0_95.txt https://builds.apache.org/job/PreCommit-HDFS-Build/14886/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-HDFS-Build/14886/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/14886/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/14886/console Powered by Apache Yetus 0.2.0 http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks for your comment, Andrew Wang. I was aware of the thread local statistics data structure, and was in favor of following the same approach. The new operation map is still per-thread. The ConcurrentHashMap was used because when aggregating, we have to make sure the map should not be modified. It's functionality is similar to the "volatile" keyword for other primitive statistic data.

          Anyway, I will revise the code and will update the patch if ConcurrentHashMap turns out unnecessary, for the sake of performance. Before that, the next patch will firstly resolve the conflicts from trunk because of HDFS-9579.

          Show
          liuml07 Mingliang Liu added a comment - Thanks for your comment, Andrew Wang . I was aware of the thread local statistics data structure, and was in favor of following the same approach. The new operation map is still per-thread. The ConcurrentHashMap was used because when aggregating, we have to make sure the map should not be modified. It's functionality is similar to the "volatile" keyword for other primitive statistic data. Anyway, I will revise the code and will update the patch if ConcurrentHashMap turns out unnecessary, for the sake of performance. Before that, the next patch will firstly resolve the conflicts from trunk because of HDFS-9579 .
          Hide
          andrew.wang Andrew Wang added a comment -

          Hi Mingliang Liu, we spent some effort optimizing the Statistics data structure to make it per-thread to avoid synchronization overheads, it showed up for apps like Spark that do multithreaded FileSystem access. I noticed this change simply uses a ConcurrentHashMap. Can we please stick with the per-thread counters and aggregation style?

          Show
          andrew.wang Andrew Wang added a comment - Hi Mingliang Liu , we spent some effort optimizing the Statistics data structure to make it per-thread to avoid synchronization overheads, it showed up for apps like Spark that do multithreaded FileSystem access. I noticed this change simply uses a ConcurrentHashMap. Can we please stick with the per-thread counters and aggregation style?
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks for creating this jira, Ram Venkatesh.

          The v0 patch is the initial effort according to the offline discussion with Jitendra Nath Pandey. More specifically, it:

          1. Creates enum OpStatistic in the FileSystem#Statistics class and supporting methods
          2. Updates the enum for every operation call in DistributedFileSystem
          3. Tries to cover S3A file system, but probably not correct
          4. Updates the unit test for testing this

          Any comment /discussion is welcome.

          Show
          liuml07 Mingliang Liu added a comment - Thanks for creating this jira, Ram Venkatesh . The v0 patch is the initial effort according to the offline discussion with Jitendra Nath Pandey . More specifically, it: Creates enum OpStatistic in the FileSystem#Statistics class and supporting methods Updates the enum for every operation call in DistributedFileSystem Tries to cover S3A file system, but probably not correct Updates the unit test for testing this Any comment /discussion is welcome.

            People

            • Assignee:
              liuml07 Mingliang Liu
              Reporter:
              venkateshrin Ram Venkatesh
            • Votes:
              0 Vote for this issue
              Watchers:
              14 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development