Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.9.0, 3.0.0-alpha2
    • Component/s: common
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Add a new instrumented read-write lock in hadoop common, so that the HDFS-9668 can use this to improve the locking in FsDatasetImpl

      1. HADOOP-13702-V10.patch
        42 kB
        Jingcheng Du
      2. HADOOP-13702-V6.patch
        22 kB
        Jingcheng Du
      3. HADOOP-13702-V7.patch
        41 kB
        Jingcheng Du
      4. HADOOP-13702-V8.patch
        42 kB
        Jingcheng Du
      5. HADOOP-13702-V9.patch
        42 kB
        Jingcheng Du
      6. HDFS-10924.patch
        20 kB
        Jingcheng Du
      7. HDFS-10924-2.patch
        21 kB
        Jingcheng Du
      8. HDFS-10924-3.patch
        21 kB
        Jingcheng Du
      9. HDFS-10924-4.patch
        22 kB
        Jingcheng Du
      10. HDFS-10924-5.patch
        22 kB
        Jingcheng Du

        Issue Links

          Activity

          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Upload the 1st patch.
          Add instrumented and auto closeable read-write locks to hdfs common.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Upload the 1st patch. Add instrumented and auto closeable read-write locks to hdfs common.
          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 7m 8s trunk passed
          +1 compile 6m 51s trunk passed
          +1 checkstyle 0m 22s trunk passed
          +1 mvnsite 0m 55s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 17s trunk passed
          +1 javadoc 0m 43s trunk passed
          +1 mvninstall 0m 36s the patch passed
          +1 compile 6m 44s the patch passed
          +1 javac 6m 44s the patch passed
          -0 checkstyle 0m 22s hadoop-common-project/hadoop-common: The patch generated 5 new + 0 unchanged - 0 fixed = 5 total (was 0)
          +1 mvnsite 0m 55s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 30s hadoop-common-project/hadoop-common generated 4 new + 0 unchanged - 0 fixed = 4 total (was 0)
          +1 javadoc 0m 45s the patch passed
          -1 unit 6m 58s hadoop-common in the patch failed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          36m 49s



          Reason Tests
          FindBugs module:hadoop-common-project/hadoop-common
            Class org.apache.hadoop.util.InstrumentedReadLock defines non-transient non-serializable instance field clock In InstrumentedReadLock.java:instance field clock In InstrumentedReadLock.java
            Class org.apache.hadoop.util.InstrumentedReadLock defines non-transient non-serializable instance field readLockHeldTimeStamp In InstrumentedReadLock.java:instance field readLockHeldTimeStamp In InstrumentedReadLock.java
            org.apache.hadoop.util.InstrumentedReadLock$1 stored into non-transient field InstrumentedReadLock.readLockHeldTimeStamp At InstrumentedReadLock.java:InstrumentedReadLock.readLockHeldTimeStamp At InstrumentedReadLock.java:[line 66]
            Class org.apache.hadoop.util.InstrumentedWriteLock defines non-transient non-serializable instance field clock In InstrumentedWriteLock.java:instance field clock In InstrumentedWriteLock.java
          Failed junit tests hadoop.net.TestClusterTopology



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10924
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830874/HDFS-10924.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 32d8ddfeb110 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 47f8092
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16925/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16925/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16925/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16925/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16925/console
          Powered by Apache Yetus 0.4.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 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 7m 8s trunk passed +1 compile 6m 51s trunk passed +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 55s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 17s trunk passed +1 javadoc 0m 43s trunk passed +1 mvninstall 0m 36s the patch passed +1 compile 6m 44s the patch passed +1 javac 6m 44s the patch passed -0 checkstyle 0m 22s hadoop-common-project/hadoop-common: The patch generated 5 new + 0 unchanged - 0 fixed = 5 total (was 0) +1 mvnsite 0m 55s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 30s hadoop-common-project/hadoop-common generated 4 new + 0 unchanged - 0 fixed = 4 total (was 0) +1 javadoc 0m 45s the patch passed -1 unit 6m 58s hadoop-common in the patch failed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 36m 49s Reason Tests FindBugs module:hadoop-common-project/hadoop-common   Class org.apache.hadoop.util.InstrumentedReadLock defines non-transient non-serializable instance field clock In InstrumentedReadLock.java:instance field clock In InstrumentedReadLock.java   Class org.apache.hadoop.util.InstrumentedReadLock defines non-transient non-serializable instance field readLockHeldTimeStamp In InstrumentedReadLock.java:instance field readLockHeldTimeStamp In InstrumentedReadLock.java   org.apache.hadoop.util.InstrumentedReadLock$1 stored into non-transient field InstrumentedReadLock.readLockHeldTimeStamp At InstrumentedReadLock.java:InstrumentedReadLock.readLockHeldTimeStamp At InstrumentedReadLock.java: [line 66]   Class org.apache.hadoop.util.InstrumentedWriteLock defines non-transient non-serializable instance field clock In InstrumentedWriteLock.java:instance field clock In InstrumentedWriteLock.java Failed junit tests hadoop.net.TestClusterTopology Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10924 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12830874/HDFS-10924.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 32d8ddfeb110 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 47f8092 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16925/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16925/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/16925/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16925/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16925/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Remove some unnecessary classes, add unit tests, and fix the warnings in checkStyle and findBugs.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Remove some unnecessary classes, add unit tests, and fix the warnings in checkStyle and findBugs.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment - - edited

          Hi Arpit Agarwal, I will have a vocation in the next whole week, and will resume my work on Oct 10. I have to refresh the patch according to comments at that time. So sorry for the inconvenience.
          The new patch V2 is uploaded according to the Hadoop QA result, pleas kindly take a look if it is okay to commit. Thanks a lot for your review and comments!

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - - edited Hi Arpit Agarwal , I will have a vocation in the next whole week, and will resume my work on Oct 10. I have to refresh the patch according to comments at that time. So sorry for the inconvenience. The new patch V2 is uploaded according to the Hadoop QA result, pleas kindly take a look if it is okay to commit. Thanks a lot for your review and comments!
          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.
          +1 mvninstall 6m 52s trunk passed
          +1 compile 6m 51s trunk passed
          +1 checkstyle 0m 23s trunk passed
          +1 mvnsite 0m 56s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 20s trunk passed
          +1 javadoc 0m 43s trunk passed
          +1 mvninstall 0m 36s the patch passed
          +1 compile 6m 46s the patch passed
          +1 javac 6m 46s the patch passed
          -0 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 21 new + 0 unchanged - 0 fixed = 21 total (was 0)
          +1 mvnsite 0m 54s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 27s the patch passed
          +1 javadoc 0m 44s the patch passed
          +1 unit 7m 47s hadoop-common in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          37m 30s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10924
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831080/HDFS-10924-2.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux bb9aa74c03b1 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 / 10be459
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16943/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16943/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16943/console
          Powered by Apache Yetus 0.4.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 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. +1 mvninstall 6m 52s trunk passed +1 compile 6m 51s trunk passed +1 checkstyle 0m 23s trunk passed +1 mvnsite 0m 56s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 20s trunk passed +1 javadoc 0m 43s trunk passed +1 mvninstall 0m 36s the patch passed +1 compile 6m 46s the patch passed +1 javac 6m 46s the patch passed -0 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 21 new + 0 unchanged - 0 fixed = 21 total (was 0) +1 mvnsite 0m 54s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 27s the patch passed +1 javadoc 0m 44s the patch passed +1 unit 7m 47s hadoop-common in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 37m 30s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10924 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831080/HDFS-10924-2.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux bb9aa74c03b1 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 / 10be459 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16943/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16943/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16943/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Upload a new patch V3 to fix the warnings in check style.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Upload a new patch V3 to fix the warnings in check style.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 23s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 9m 38s trunk passed
          +1 compile 9m 2s trunk passed
          +1 checkstyle 0m 30s trunk passed
          +1 mvnsite 1m 16s trunk passed
          +1 mvneclipse 0m 16s trunk passed
          +1 findbugs 1m 39s trunk passed
          +1 javadoc 0m 56s trunk passed
          +1 mvninstall 0m 51s the patch passed
          +1 compile 7m 16s the patch passed
          +1 javac 7m 16s the patch passed
          +1 checkstyle 0m 22s the patch passed
          +1 mvnsite 0m 53s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 27s the patch passed
          +1 javadoc 0m 44s the patch passed
          +1 unit 8m 5s hadoop-common in the patch passed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          44m 35s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10924
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831496/HDFS-10924-3.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 98267074e7bd 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / f61e3d1
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16996/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16996/console
          Powered by Apache Yetus 0.4.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 23s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 9m 38s trunk passed +1 compile 9m 2s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 1m 16s trunk passed +1 mvneclipse 0m 16s trunk passed +1 findbugs 1m 39s trunk passed +1 javadoc 0m 56s trunk passed +1 mvninstall 0m 51s the patch passed +1 compile 7m 16s the patch passed +1 javac 7m 16s the patch passed +1 checkstyle 0m 22s the patch passed +1 mvnsite 0m 53s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 27s the patch passed +1 javadoc 0m 44s the patch passed +1 unit 8m 5s hadoop-common in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 44m 35s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10924 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12831496/HDFS-10924-3.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 98267074e7bd 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f61e3d1 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16996/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16996/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Jingcheng Du for the nice work here, and on the related HDFS-9668.

          Some comments on patch 3, looks good overall:
          AutoCloseableReadWriteLockWrapper:

          • Are the constructors of AutoCloseableReadLock and AutoCloseableWriteLock intentionally public? I'm having a hard time coming up with a scenario of this.
          • Maybe we could update the above to constructors to accept the lock parameter. This is also more consistent with what the new instrumented lock class does. (as well as java's ReentrantReadWriteLock class)

          InstrumentedAutoCloseableReadWriteLockWrapper:

          • Please check input variables on InstrumentedAutoCloseableReadWriteLockWrapper's constructor. (e.g. time > 0, logger not null. Do we allow null name?)
          • Personally feels the following change would be more readable:
                if (lockWarningThresholdMs - lockHeldTime < 0) {
                    ...
                    now = clock.monotonicNow();
                    localLastLogTs = lastLogTimestamp.get();
                    long deltaSinceLastLog = now - localLastLogTs;
                    // check should print log or not
                    if (deltaSinceLastLog - minLoggingGapMs < 0) {
                      warningsSuppressed.incrementAndGet();
                      return;
                    }
            

            to

                if (lockHeldTime > lockWarningThresholdMs) {
                    ....
                    now = clock.monotonicNow();
                    localLastLogTs = lastLogTimestamp.get();
                    if (now - localLastLogTs < minLoggingGapMs) {
                      warningsSuppressed.incrementAndGet();
                      return;
                    }
            
          • Feels like a StringBuilder would be more readable and efficient in logWarning, than the current 3 concatenation.
          Show
          xiaochen Xiao Chen added a comment - Thanks Jingcheng Du for the nice work here, and on the related HDFS-9668 . Some comments on patch 3, looks good overall: AutoCloseableReadWriteLockWrapper : Are the constructors of AutoCloseableReadLock and AutoCloseableWriteLock intentionally public? I'm having a hard time coming up with a scenario of this. Maybe we could update the above to constructors to accept the lock parameter. This is also more consistent with what the new instrumented lock class does. (as well as java's ReentrantReadWriteLock class) InstrumentedAutoCloseableReadWriteLockWrapper : Please check input variables on InstrumentedAutoCloseableReadWriteLockWrapper 's constructor. (e.g. time > 0, logger not null. Do we allow null name?) Personally feels the following change would be more readable: if (lockWarningThresholdMs - lockHeldTime < 0) { ... now = clock.monotonicNow(); localLastLogTs = lastLogTimestamp.get(); long deltaSinceLastLog = now - localLastLogTs; // check should print log or not if (deltaSinceLastLog - minLoggingGapMs < 0) { warningsSuppressed.incrementAndGet(); return ; } to if (lockHeldTime > lockWarningThresholdMs) { .... now = clock.monotonicNow(); localLastLogTs = lastLogTimestamp.get(); if (now - localLastLogTs < minLoggingGapMs) { warningsSuppressed.incrementAndGet(); return ; } Feels like a StringBuilder would be more readable and efficient in logWarning , than the current 3 concatenation.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment - - edited

          Thanks a lot Xiao Chen for the comments.

          Are the constructors of AutoCloseableReadLock and AutoCloseableWriteLock intentionally public? I'm having a hard time coming up with a scenario of this.

          Actually no, I have changed them to package private.

          Feels like a StringBuilder would be more readable and efficient in logWarning, than the current 3 concatenation.

          For the log warning, these three strings can be concatenated into one after the compiling, this should be fine.
          But yes, the StringBuilder is much more efficient than String.format, do we need to use StringBuilder instead of String.format, or it's okay to use String.format as now?

          For others, they are good ones, I will fix them in the next patch V4. Thanks!

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - - edited Thanks a lot Xiao Chen for the comments. Are the constructors of AutoCloseableReadLock and AutoCloseableWriteLock intentionally public? I'm having a hard time coming up with a scenario of this. Actually no, I have changed them to package private. Feels like a StringBuilder would be more readable and efficient in logWarning, than the current 3 concatenation. For the log warning, these three strings can be concatenated into one after the compiling, this should be fine. But yes, the StringBuilder is much more efficient than String.format, do we need to use StringBuilder instead of String.format, or it's okay to use String.format as now? For others, they are good ones, I will fix them in the next patch V4. Thanks!
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          A new patch V4 is uploaded.
          Please take a look if it is okay to commit. Thanks a lot.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - A new patch V4 is uploaded. Please take a look if it is okay to commit. Thanks a lot.
          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 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 3s trunk passed
          +1 compile 6m 48s trunk passed
          +1 checkstyle 0m 22s trunk passed
          +1 mvnsite 0m 56s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 16s trunk passed
          +1 javadoc 0m 39s trunk passed
          +1 mvninstall 0m 35s the patch passed
          +1 compile 6m 38s the patch passed
          +1 javac 6m 38s the patch passed
          +1 checkstyle 0m 21s the patch passed
          +1 mvnsite 0m 52s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 24s the patch passed
          +1 javadoc 0m 41s the patch passed
          +1 unit 7m 16s hadoop-common in the patch passed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          36m 39s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10924
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832408/HDFS-10924-4.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 985da11e6788 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / bea004e
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17079/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17079/console
          Powered by Apache Yetus 0.4.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 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 3s trunk passed +1 compile 6m 48s trunk passed +1 checkstyle 0m 22s trunk passed +1 mvnsite 0m 56s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 16s trunk passed +1 javadoc 0m 39s trunk passed +1 mvninstall 0m 35s the patch passed +1 compile 6m 38s the patch passed +1 javac 6m 38s the patch passed +1 checkstyle 0m 21s the patch passed +1 mvnsite 0m 52s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 24s the patch passed +1 javadoc 0m 41s the patch passed +1 unit 7m 16s hadoop-common in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 36m 39s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10924 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832408/HDFS-10924-4.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 985da11e6788 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12 17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / bea004e Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/17079/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HDFS-Build/17079/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          chris.douglas Chris Douglas added a comment -

          Personally feels the following change would be more readable: [snip]

          This follows guidance for System.nanoTime.

          The HDFS InstrumentedLock class contains nearly identical functionality. If Common will introduce other instrumented locks, then this should replace what's in HDFS.

          AutoCloseableReadLock and AutoCloseableReadLock are redundant with the base impl, mod isLocked (which returns isWriteLocked for AutoCloseableReadLock). As in o.a.h.hdfs.InstrumentedLock, this should not build around AutoCloseableLock as the base API, but the classes in java.util.concurrent.locks. Specifically, an InstrumentedReadWriteLock should return instrumented locks for readLock() and writeLock(). It's up to the caller to wrap what's returned in an AutoCloseableLock, if desired.

          Show
          chris.douglas Chris Douglas added a comment - Personally feels the following change would be more readable: [snip] This follows guidance for System.nanoTime . The HDFS InstrumentedLock class contains nearly identical functionality. If Common will introduce other instrumented locks, then this should replace what's in HDFS. AutoCloseableReadLock and AutoCloseableReadLock are redundant with the base impl, mod isLocked (which returns isWriteLocked for AutoCloseableReadLock ). As in o.a.h.hdfs.InstrumentedLock , this should not build around AutoCloseableLock as the base API, but the classes in java.util.concurrent.locks . Specifically, an InstrumentedReadWriteLock should return instrumented locks for readLock() and writeLock() . It's up to the caller to wrap what's returned in an AutoCloseableLock , if desired.
          Hide
          chris.douglas Chris Douglas added a comment -

          If this is being committed to common, the issue should be moved to the COMMON project.

          Sorry, I'd missed that this was moving code from HDFS-9668. I agree with Arpit Agarwal's comment on reducing the variety of lock instrumentation classes.

          Show
          chris.douglas Chris Douglas added a comment - If this is being committed to common, the issue should be moved to the COMMON project. Sorry, I'd missed that this was moving code from HDFS-9668 . I agree with Arpit Agarwal 's comment on reducing the variety of lock instrumentation classes.
          Hide
          drankye Kai Zheng added a comment -

          Thanks Chris Douglas for the comments. I just moved it to the common side.

          Show
          drankye Kai Zheng added a comment - Thanks Chris Douglas for the comments. I just moved it to the common side.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 8s trunk passed
          +1 compile 7m 14s trunk passed
          +1 checkstyle 0m 24s trunk passed
          +1 mvnsite 0m 59s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 22s trunk passed
          +1 javadoc 0m 44s trunk passed
          +1 mvninstall 0m 37s the patch passed
          +1 compile 7m 16s the patch passed
          +1 javac 7m 16s the patch passed
          +1 checkstyle 0m 24s the patch passed
          +1 mvnsite 0m 56s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 32s the patch passed
          +1 javadoc 0m 43s the patch passed
          -1 unit 10m 14s hadoop-common in the patch failed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          42m 4s



          Reason Tests
          Failed junit tests hadoop.ipc.TestIPC



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13702
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832408/HDFS-10924-4.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 45592cf5a8f0 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 / af50da3
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10718/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10718/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10718/console
          Powered by Apache Yetus 0.4.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 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 8s trunk passed +1 compile 7m 14s trunk passed +1 checkstyle 0m 24s trunk passed +1 mvnsite 0m 59s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 22s trunk passed +1 javadoc 0m 44s trunk passed +1 mvninstall 0m 37s the patch passed +1 compile 7m 16s the patch passed +1 javac 7m 16s the patch passed +1 checkstyle 0m 24s the patch passed +1 mvnsite 0m 56s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 32s the patch passed +1 javadoc 0m 43s the patch passed -1 unit 10m 14s hadoop-common in the patch failed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 42m 4s Reason Tests Failed junit tests hadoop.ipc.TestIPC Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13702 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832408/HDFS-10924-4.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 45592cf5a8f0 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 / af50da3 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10718/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10718/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10718/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Thanks a lot for the great comments Chris Douglas.
          I upload a new patch V5 to address them.
          Since both ReadLock/WriteLock and InstrumentedLock are not interfaces, I have to implement InstrumentedReadLock/InstrumentedWriteLock to extend ReadLock/WriteLock instead of InstrumentedLock, and this makes some duplicated code.
          In order to align the InstrumentedReadLock/WriteLock together with InstrumentedLock, I moved them to hdfs project instead of common.
          Please take a look if anything needs to be improved. Thanks a lot.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Thanks a lot for the great comments Chris Douglas . I upload a new patch V5 to address them. Since both ReadLock/WriteLock and InstrumentedLock are not interfaces, I have to implement InstrumentedReadLock/InstrumentedWriteLock to extend ReadLock/WriteLock instead of InstrumentedLock, and this makes some duplicated code. In order to align the InstrumentedReadLock/WriteLock together with InstrumentedLock, I moved them to hdfs project instead of common. Please take a look if anything needs to be improved. Thanks a lot.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 14s Maven dependency ordering for branch
          +1 mvninstall 6m 35s trunk passed
          +1 compile 6m 47s trunk passed
          +1 checkstyle 1m 26s trunk passed
          +1 mvnsite 1m 49s trunk passed
          +1 mvneclipse 0m 26s trunk passed
          +1 findbugs 3m 8s trunk passed
          +1 javadoc 1m 25s trunk passed
          0 mvndep 0m 16s Maven dependency ordering for patch
          +1 mvninstall 1m 26s the patch passed
          +1 compile 6m 44s the patch passed
          +1 javac 6m 44s the patch passed
          -0 checkstyle 1m 28s root: The patch generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)
          +1 mvnsite 1m 51s the patch passed
          +1 mvneclipse 0m 33s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 27s the patch passed
          +1 javadoc 1m 32s the patch passed
          -1 unit 17m 11s hadoop-common in the patch failed.
          -1 unit 61m 7s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 29s The patch does not generate ASF License warnings.
          141m 24s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.datanode.TestLargeBlockReport
          Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13702
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832445/HDFS-10924-5.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 7f7b131b780f 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 / af50da3
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10719/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10719/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10719/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10719/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10719/console
          Powered by Apache Yetus 0.4.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 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 14s Maven dependency ordering for branch +1 mvninstall 6m 35s trunk passed +1 compile 6m 47s trunk passed +1 checkstyle 1m 26s trunk passed +1 mvnsite 1m 49s trunk passed +1 mvneclipse 0m 26s trunk passed +1 findbugs 3m 8s trunk passed +1 javadoc 1m 25s trunk passed 0 mvndep 0m 16s Maven dependency ordering for patch +1 mvninstall 1m 26s the patch passed +1 compile 6m 44s the patch passed +1 javac 6m 44s the patch passed -0 checkstyle 1m 28s root: The patch generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) +1 mvnsite 1m 51s the patch passed +1 mvneclipse 0m 33s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 27s the patch passed +1 javadoc 1m 32s the patch passed -1 unit 17m 11s hadoop-common in the patch failed. -1 unit 61m 7s hadoop-hdfs in the patch failed. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 141m 24s Reason Tests Failed junit tests hadoop.hdfs.server.datanode.TestLargeBlockReport Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13702 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832445/HDFS-10924-5.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 7f7b131b780f 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 / af50da3 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10719/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10719/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10719/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10719/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10719/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Re-attach the patch V5.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Re-attach the patch V5.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 25s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 8m 44s trunk passed
          +1 compile 0m 58s trunk passed
          +1 checkstyle 0m 30s trunk passed
          +1 mvnsite 1m 4s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 1m 58s trunk passed
          +1 javadoc 0m 46s trunk passed
          +1 mvninstall 0m 56s the patch passed
          +1 compile 0m 53s the patch passed
          +1 javac 0m 53s the patch passed
          +1 checkstyle 0m 29s the patch passed
          +1 mvnsite 1m 2s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 3s the patch passed
          +1 javadoc 0m 42s the patch passed
          -1 unit 76m 14s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          99m 45s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.datanode.TestLargeBlockReport
            hadoop.hdfs.server.namenode.ha.TestEditLogTailer
            hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure
            hadoop.hdfs.server.datanode.TestDataNodeHotSwapVolumes
            hadoop.hdfs.qjournal.client.TestQuorumJournalManager



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13702
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832485/HDFS-10924-5.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 19c218730351 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / cef61d5
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10721/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10721/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10721/console
          Powered by Apache Yetus 0.4.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 25s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 8m 44s trunk passed +1 compile 0m 58s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 1m 4s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 1m 58s trunk passed +1 javadoc 0m 46s trunk passed +1 mvninstall 0m 56s the patch passed +1 compile 0m 53s the patch passed +1 javac 0m 53s the patch passed +1 checkstyle 0m 29s the patch passed +1 mvnsite 1m 2s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 3s the patch passed +1 javadoc 0m 42s the patch passed -1 unit 76m 14s hadoop-hdfs in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 99m 45s Reason Tests Failed junit tests hadoop.hdfs.server.datanode.TestLargeBlockReport   hadoop.hdfs.server.namenode.ha.TestEditLogTailer   hadoop.hdfs.server.datanode.TestDataNodeVolumeFailure   hadoop.hdfs.server.datanode.TestDataNodeHotSwapVolumes   hadoop.hdfs.qjournal.client.TestQuorumJournalManager Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13702 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832485/HDFS-10924-5.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 19c218730351 3.13.0-96-generic #143-Ubuntu SMP Mon Aug 29 20:15:20 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / cef61d5 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10721/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10721/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10721/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          This follows guidance for System.nanoTime.

          Thanks Chris Douglas for the good catch!

          Also, seems the fix is moved back to HDFS, we should make sure the component is consistent with the final change before check-in.
          Jingcheng Du thanks for revving. Why the new InstrumentedReadLock and InstrumentedWriteLock can't reuse InstrumentedLock?

          Show
          xiaochen Xiao Chen added a comment - This follows guidance for System.nanoTime. Thanks Chris Douglas for the good catch! Also, seems the fix is moved back to HDFS, we should make sure the component is consistent with the final change before check-in. Jingcheng Du thanks for revving. Why the new InstrumentedReadLock and InstrumentedWriteLock can't reuse InstrumentedLock ?
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Thanks for the comments Xiao Chen!

          InstrumentedLock which is also in branch-2.8 is now in HDFS, I moved the InstrumentedReadLock and InstrumentedWriteLock back to hdfs to align them with InstrumentedLock together. I can move the read-write lock to COMMON in the next patch.
          It is true to extend InstrumentedLock can avoid the code duplicated, but the read-write lock code cannot be in COMMON anymore since InstrumentedLock is in HDFS.
          And is it more straightforward if we use a InstrumentedReadLock/InstrumentedWriteLock as ReadLock/WriteLock, and use readLock/writeLock methods in InstrumentedReadWriteLock to get them?
          Please advise. Thanks a lot.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Thanks for the comments Xiao Chen ! InstrumentedLock which is also in branch-2.8 is now in HDFS, I moved the InstrumentedReadLock and InstrumentedWriteLock back to hdfs to align them with InstrumentedLock together. I can move the read-write lock to COMMON in the next patch. It is true to extend InstrumentedLock can avoid the code duplicated, but the read-write lock code cannot be in COMMON anymore since InstrumentedLock is in HDFS. And is it more straightforward if we use a InstrumentedReadLock/InstrumentedWriteLock as ReadLock/WriteLock, and use readLock/writeLock methods in InstrumentedReadWriteLock to get them? Please advise. Thanks a lot.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Upload a new patch V6, which moves the instrumented read-write locks to COMMON.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Upload a new patch V6, which moves the instrumented read-write locks to COMMON.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 20s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 59s trunk passed
          +1 compile 7m 11s trunk passed
          +1 checkstyle 0m 24s trunk passed
          +1 mvnsite 0m 58s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 21s trunk passed
          +1 javadoc 0m 43s trunk passed
          +1 mvninstall 0m 37s the patch passed
          +1 compile 7m 4s the patch passed
          +1 javac 7m 4s the patch passed
          +1 checkstyle 0m 23s the patch passed
          +1 mvnsite 0m 53s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 30s the patch passed
          +1 javadoc 0m 42s the patch passed
          +1 unit 8m 37s hadoop-common in the patch passed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          39m 57s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13702
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832604/HADOOP-13702-V6.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 1b4791f28756 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 / 96b1266
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10727/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10727/console
          Powered by Apache Yetus 0.4.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 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 59s trunk passed +1 compile 7m 11s trunk passed +1 checkstyle 0m 24s trunk passed +1 mvnsite 0m 58s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 21s trunk passed +1 javadoc 0m 43s trunk passed +1 mvninstall 0m 37s the patch passed +1 compile 7m 4s the patch passed +1 javac 7m 4s the patch passed +1 checkstyle 0m 23s the patch passed +1 mvnsite 0m 53s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 30s the patch passed +1 javadoc 0m 42s the patch passed +1 unit 8m 37s hadoop-common in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 39m 57s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13702 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12832604/HADOOP-13702-V6.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 1b4791f28756 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 / 96b1266 Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10727/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10727/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Hi guys, any comments for the latest patch? All of them are appreciated.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Hi guys, any comments for the latest patch? All of them are appreciated.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Jingcheng Du for revving.
          The separate readlock / writelock looks good.

          I feel the various code duplication should still be improved. IMHO we could just have an instrumentation class - what type of lock it's instrumenting should be separated.

          Also, if I read Chris Douglas's comment correctly:

          The HDFS InstrumentedLock class contains nearly identical functionality. If Common will introduce other instrumented locks, then this should replace what's in HDFS.

          The proposal was, if we put this in common, we should replace the existing InstrumentedLock in hdfs, instead of copy-pasting. So we don't have to 'align' with that class in HDFS.

          Would also like to hear what others think.

          Show
          xiaochen Xiao Chen added a comment - Thanks Jingcheng Du for revving. The separate readlock / writelock looks good. I feel the various code duplication should still be improved. IMHO we could just have an instrumentation class - what type of lock it's instrumenting should be separated. Also, if I read Chris Douglas 's comment correctly: The HDFS InstrumentedLock class contains nearly identical functionality. If Common will introduce other instrumented locks, then this should replace what's in HDFS. The proposal was, if we put this in common, we should replace the existing InstrumentedLock in hdfs, instead of copy-pasting. So we don't have to 'align' with that class in HDFS. Would also like to hear what others think.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Thanks Xiao Chen! I have some misunderstanding on the returned type of readLock and writeLock in ReadWriteLock, let me post another patch to fix this

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Thanks Xiao Chen ! I have some misunderstanding on the returned type of readLock and writeLock in ReadWriteLock , let me post another patch to fix this
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Upload a new patch V7 to address the comments from Xiao Chen. Thanks.

          1. Move InstrumentedLock and TestInstrumentedLock to COMMON. Please be noted, these classes are also in branch 2.8.0, and they are in HDFS there.
          2. Use InstrumentedReadLock and InstrumentedWriteLock to extend InstrumentedLock, and remove the duplicated code.
            Hi all, please take a look if this latest patch V7 is okay. Thanks.
          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Upload a new patch V7 to address the comments from Xiao Chen . Thanks. Move InstrumentedLock and TestInstrumentedLock to COMMON. Please be noted, these classes are also in branch 2.8.0, and they are in HDFS there. Use InstrumentedReadLock and InstrumentedWriteLock to extend InstrumentedLock, and remove the duplicated code. Hi all, please take a look if this latest patch V7 is okay. Thanks.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s 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 32s Maven dependency ordering for branch
          +1 mvninstall 7m 3s trunk passed
          +1 compile 7m 42s trunk passed
          +1 checkstyle 1m 31s trunk passed
          +1 mvnsite 1m 52s trunk passed
          +1 mvneclipse 0m 26s trunk passed
          +1 findbugs 3m 11s trunk passed
          +1 javadoc 1m 27s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 1m 29s the patch passed
          +1 compile 7m 10s the patch passed
          +1 javac 7m 10s the patch passed
          -0 checkstyle 1m 32s root: The patch generated 12 new + 80 unchanged - 0 fixed = 92 total (was 80)
          +1 mvnsite 1m 55s the patch passed
          +1 mvneclipse 0m 31s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 25s the patch passed
          +1 javadoc 1m 28s the patch passed
          +1 unit 7m 31s hadoop-common in the patch passed.
          -1 unit 65m 6s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 41s The patch does not generate ASF License warnings.
          139m 25s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.blockmanagement.TestRBWBlockInvalidation



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13702
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833865/HADOOP-13702-V7.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 25d231644200 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / b61fb26
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10820/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10820/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10820/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10820/console
          Powered by Apache Yetus 0.4.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 17s 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 32s Maven dependency ordering for branch +1 mvninstall 7m 3s trunk passed +1 compile 7m 42s trunk passed +1 checkstyle 1m 31s trunk passed +1 mvnsite 1m 52s trunk passed +1 mvneclipse 0m 26s trunk passed +1 findbugs 3m 11s trunk passed +1 javadoc 1m 27s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 29s the patch passed +1 compile 7m 10s the patch passed +1 javac 7m 10s the patch passed -0 checkstyle 1m 32s root: The patch generated 12 new + 80 unchanged - 0 fixed = 92 total (was 80) +1 mvnsite 1m 55s the patch passed +1 mvneclipse 0m 31s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 25s the patch passed +1 javadoc 1m 28s the patch passed +1 unit 7m 31s hadoop-common in the patch passed. -1 unit 65m 6s hadoop-hdfs in the patch failed. +1 asflicense 0m 41s The patch does not generate ASF License warnings. 139m 25s Reason Tests Failed junit tests hadoop.hdfs.server.blockmanagement.TestRBWBlockInvalidation Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13702 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12833865/HADOOP-13702-V7.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 25d231644200 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / b61fb26 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10820/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10820/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10820/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10820/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Upload a new patch V8 to fix the warnings in checkstyle.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Upload a new patch V8 to fix the warnings in checkstyle.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks for the new patch Jingcheng Du! Looks pretty nice, I think we're close.

          Some more comments:

          • We could modify InstrumentedReadLock#tryLock to do it in the OO way. That is, make recordLockAcquireTimestamp a base-class method, and override it in the readlock. This way we don't have to override the tryLock which mostly duplicates the base class logic.
          • Similar to unlock and check
          • Suggest to put more details in javadoc of the 3 new classes, and reference to InstrumentedLock. Maybe something like
            This is a wrap class of a {@link InstrumentedLock} and a WriteLock.
            

          Otherwise LGTM.

          Show
          xiaochen Xiao Chen added a comment - Thanks for the new patch Jingcheng Du ! Looks pretty nice, I think we're close. Some more comments: We could modify InstrumentedReadLock#tryLock to do it in the OO way. That is, make recordLockAcquireTimestamp a base-class method, and override it in the readlock. This way we don't have to override the tryLock which mostly duplicates the base class logic. Similar to unlock and check Suggest to put more details in javadoc of the 3 new classes, and reference to InstrumentedLock . Maybe something like This is a wrap class of a {@link InstrumentedLock} and a WriteLock. Otherwise LGTM.
          Hide
          chris.douglas Chris Douglas added a comment -

          Thank you for all the refinements, Jingcheng Du. This is looking good.

          • The constructors accepting Timer can be package-private and annotated as @VisibleForTesting.
          • InstrumentedReadWriteLock could accept an instance of ReadWriteLock instead of creating one. Similarly, the read/write locks could accept a ReadWriteLock in their constructor. IRWL could also have the convenience constructor, as written.
          • InstrumentedReadWriteLock fields can be final
          • Shouldn't the ThreadLocal<Long> readLockHeldTimeStamp field be final, not transient?
          • Is InstrumentedLock unused in HDFS? I expected to see package changes in the HDFS project for the imports to resolve.
          Show
          chris.douglas Chris Douglas added a comment - Thank you for all the refinements, Jingcheng Du . This is looking good. The constructors accepting Timer can be package-private and annotated as @VisibleForTesting . InstrumentedReadWriteLock could accept an instance of ReadWriteLock instead of creating one. Similarly, the read/write locks could accept a ReadWriteLock in their constructor. IRWL could also have the convenience constructor, as written. InstrumentedReadWriteLock fields can be final Shouldn't the ThreadLocal<Long> readLockHeldTimeStamp field be final, not transient? Is InstrumentedLock unused in HDFS? I expected to see package changes in the HDFS project for the imports to resolve.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Thanks a lot Xiao Chen! All comments are addressed, will post a new patch soon.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Thanks a lot Xiao Chen ! All comments are addressed, will post a new patch soon.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Thanks a lot Chris Douglas.

          Shouldn't the ThreadLocal<Long> readLockHeldTimeStamp field be final, not transient?

          Sorry for the careless, that was an old code for extending ReadLock which is serializable, I only removed the readObject method but forgot this, so sorry.

          InstrumentedReadWriteLock could accept an instance of ReadWriteLock instead of creating one. Similarly, the read/write locks could accept a ReadWriteLock in their constructor. IRWL could also have the convenience constructor, as written.

          The instrumented read lock uses methods in ReentrantReadWriteLock for instrumentation. If we accept a ReadWriteLock in constructors i am afraid we cannot support the instrumentation in the read lock.

          Is InstrumentedLock unused in HDFS? I expected to see package changes in the HDFS project for the imports to resolve.

          Only FsDatasetImpl uses this class, and this change is included in the patch

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Thanks a lot Chris Douglas . Shouldn't the ThreadLocal<Long> readLockHeldTimeStamp field be final, not transient? Sorry for the careless, that was an old code for extending ReadLock which is serializable, I only removed the readObject method but forgot this, so sorry. InstrumentedReadWriteLock could accept an instance of ReadWriteLock instead of creating one. Similarly, the read/write locks could accept a ReadWriteLock in their constructor. IRWL could also have the convenience constructor, as written. The instrumented read lock uses methods in ReentrantReadWriteLock for instrumentation. If we accept a ReadWriteLock in constructors i am afraid we cannot support the instrumentation in the read lock. Is InstrumentedLock unused in HDFS? I expected to see package changes in the HDFS project for the imports to resolve. Only FsDatasetImpl uses this class, and this change is included in the patch
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Upload a new patch V9 to address the comments from Chris Douglas and Xiao Chen. Thanks a lot.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Upload a new patch V9 to address the comments from Chris Douglas and Xiao Chen . Thanks a lot.
          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 appears to include 3 new or modified test files.
          0 mvndep 1m 40s Maven dependency ordering for branch
          +1 mvninstall 7m 54s trunk passed
          +1 compile 8m 19s trunk passed
          +1 checkstyle 1m 37s trunk passed
          +1 mvnsite 2m 6s trunk passed
          +1 mvneclipse 0m 28s trunk passed
          +1 findbugs 3m 25s trunk passed
          +1 javadoc 1m 35s trunk passed
          0 mvndep 0m 18s Maven dependency ordering for patch
          +1 mvninstall 1m 42s the patch passed
          +1 compile 8m 12s the patch passed
          +1 javac 8m 12s the patch passed
          -0 checkstyle 1m 38s root: The patch generated 6 new + 81 unchanged - 0 fixed = 87 total (was 81)
          +1 mvnsite 2m 4s the patch passed
          +1 mvneclipse 0m 31s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 56s the patch passed
          +1 javadoc 1m 41s the patch passed
          +1 unit 8m 41s hadoop-common in the patch passed.
          -1 unit 60m 33s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 30s The patch does not generate ASF License warnings.
          140m 24s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.namenode.TestFSImage
            hadoop.hdfs.TestRenameWhileOpen
            hadoop.hdfs.server.namenode.TestAddStripedBlockInFBR



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13702
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12834594/HADOOP-13702-V9.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 2f4f9e77b245 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 / d7d87de
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10847/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10847/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10847/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10847/console
          Powered by Apache Yetus 0.4.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 15s 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 40s Maven dependency ordering for branch +1 mvninstall 7m 54s trunk passed +1 compile 8m 19s trunk passed +1 checkstyle 1m 37s trunk passed +1 mvnsite 2m 6s trunk passed +1 mvneclipse 0m 28s trunk passed +1 findbugs 3m 25s trunk passed +1 javadoc 1m 35s trunk passed 0 mvndep 0m 18s Maven dependency ordering for patch +1 mvninstall 1m 42s the patch passed +1 compile 8m 12s the patch passed +1 javac 8m 12s the patch passed -0 checkstyle 1m 38s root: The patch generated 6 new + 81 unchanged - 0 fixed = 87 total (was 81) +1 mvnsite 2m 4s the patch passed +1 mvneclipse 0m 31s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 56s the patch passed +1 javadoc 1m 41s the patch passed +1 unit 8m 41s hadoop-common in the patch passed. -1 unit 60m 33s hadoop-hdfs in the patch failed. +1 asflicense 0m 30s The patch does not generate ASF License warnings. 140m 24s Reason Tests Failed junit tests hadoop.hdfs.server.namenode.TestFSImage   hadoop.hdfs.TestRenameWhileOpen   hadoop.hdfs.server.namenode.TestAddStripedBlockInFBR Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13702 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12834594/HADOOP-13702-V9.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 2f4f9e77b245 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 / d7d87de Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10847/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10847/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10847/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10847/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Upload a new patch V10 to fix the warnings in checkstyle.
          The test failure should not be related with this patch.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Upload a new patch V10 to fix the warnings in checkstyle. The test failure should not be related with this patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 16s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 1m 41s Maven dependency ordering for branch
          +1 mvninstall 8m 8s trunk passed
          +1 compile 7m 51s trunk passed
          +1 checkstyle 1m 35s trunk passed
          +1 mvnsite 1m 49s trunk passed
          +1 mvneclipse 0m 27s trunk passed
          +1 findbugs 3m 4s trunk passed
          +1 javadoc 1m 26s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 1m 26s the patch passed
          +1 compile 6m 48s the patch passed
          +1 javac 6m 48s the patch passed
          -0 checkstyle 1m 30s root: The patch generated 6 new + 80 unchanged - 0 fixed = 86 total (was 80)
          +1 mvnsite 1m 51s the patch passed
          +1 mvneclipse 0m 32s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 29s the patch passed
          +1 javadoc 1m 32s the patch passed
          -1 unit 17m 4s hadoop-common in the patch failed.
          +1 unit 58m 3s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 29s The patch does not generate ASF License warnings.
          142m 40s



          Reason Tests
          Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13702
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12834616/HADOOP-13702-V10.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux d8e16e512e56 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 / 754cb4e
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10850/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10850/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10850/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10850/console
          Powered by Apache Yetus 0.4.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 16s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 1m 41s Maven dependency ordering for branch +1 mvninstall 8m 8s trunk passed +1 compile 7m 51s trunk passed +1 checkstyle 1m 35s trunk passed +1 mvnsite 1m 49s trunk passed +1 mvneclipse 0m 27s trunk passed +1 findbugs 3m 4s trunk passed +1 javadoc 1m 26s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 26s the patch passed +1 compile 6m 48s the patch passed +1 javac 6m 48s the patch passed -0 checkstyle 1m 30s root: The patch generated 6 new + 80 unchanged - 0 fixed = 86 total (was 80) +1 mvnsite 1m 51s the patch passed +1 mvneclipse 0m 32s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 29s the patch passed +1 javadoc 1m 32s the patch passed -1 unit 17m 4s hadoop-common in the patch failed. +1 unit 58m 3s hadoop-hdfs in the patch passed. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 142m 40s Reason Tests Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13702 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12834616/HADOOP-13702-V10.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux d8e16e512e56 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 / 754cb4e Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10850/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10850/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10850/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10850/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          Re-attach the correct patch V10.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - Re-attach the correct patch V10.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 16s Maven dependency ordering for branch
          +1 mvninstall 6m 45s trunk passed
          +1 compile 6m 53s trunk passed
          +1 checkstyle 1m 27s trunk passed
          +1 mvnsite 1m 50s trunk passed
          +1 mvneclipse 0m 27s trunk passed
          +1 findbugs 3m 7s trunk passed
          +1 javadoc 1m 26s trunk passed
          0 mvndep 0m 17s Maven dependency ordering for patch
          +1 mvninstall 1m 26s the patch passed
          +1 compile 6m 49s the patch passed
          +1 javac 6m 49s the patch passed
          +1 checkstyle 1m 30s the patch passed
          +1 mvnsite 1m 53s the patch passed
          +1 mvneclipse 0m 33s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 26s the patch passed
          +1 javadoc 1m 32s the patch passed
          +1 unit 8m 13s hadoop-common in the patch passed.
          -1 unit 58m 0s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 30s The patch does not generate ASF License warnings.
          130m 8s



          Reason Tests
          Failed junit tests hadoop.hdfs.TestDecommissionWithStriped



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13702
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12834625/HADOOP-13702-V10.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f331c688d17c 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 / 754cb4e
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10852/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10852/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10852/console
          Powered by Apache Yetus 0.4.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 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 16s Maven dependency ordering for branch +1 mvninstall 6m 45s trunk passed +1 compile 6m 53s trunk passed +1 checkstyle 1m 27s trunk passed +1 mvnsite 1m 50s trunk passed +1 mvneclipse 0m 27s trunk passed +1 findbugs 3m 7s trunk passed +1 javadoc 1m 26s trunk passed 0 mvndep 0m 17s Maven dependency ordering for patch +1 mvninstall 1m 26s the patch passed +1 compile 6m 49s the patch passed +1 javac 6m 49s the patch passed +1 checkstyle 1m 30s the patch passed +1 mvnsite 1m 53s the patch passed +1 mvneclipse 0m 33s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 26s the patch passed +1 javadoc 1m 32s the patch passed +1 unit 8m 13s hadoop-common in the patch passed. -1 unit 58m 0s hadoop-hdfs in the patch failed. +1 asflicense 0m 30s The patch does not generate ASF License warnings. 130m 8s Reason Tests Failed junit tests hadoop.hdfs.TestDecommissionWithStriped Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13702 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12834625/HADOOP-13702-V10.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f331c688d17c 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 / 754cb4e Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10852/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10852/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10852/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jingcheng.du@intel.com Jingcheng Du added a comment -

          The test failed because of "Address already in use". This should not be related with this patch.

          Show
          jingcheng.du@intel.com Jingcheng Du added a comment - The test failed because of "Address already in use". This should not be related with this patch.
          Hide
          chris.douglas Chris Douglas added a comment -

          +1

          I committed this. Thanks Jingcheng

          Show
          chris.douglas Chris Douglas added a comment - +1 I committed this. Thanks Jingcheng
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10655 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10655/)
          HADOOP-13702. Add instrumented ReadWriteLock. Contributed by Jingcheng (cdouglas: rev ae8bccd5090d8b42dae9a8e0c13a9766a7c42ecb)

          • (add) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/InstrumentedWriteLock.java
          • (delete) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestInstrumentedLock.java
          • (add) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/InstrumentedLock.java
          • (add) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/InstrumentedReadLock.java
          • (add) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/InstrumentedReadWriteLock.java
          • (add) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestInstrumentedReadWriteLock.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          • (delete) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/InstrumentedLock.java
          • (add) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestInstrumentedLock.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10655 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10655/ ) HADOOP-13702 . Add instrumented ReadWriteLock. Contributed by Jingcheng (cdouglas: rev ae8bccd5090d8b42dae9a8e0c13a9766a7c42ecb) (add) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/InstrumentedWriteLock.java (delete) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestInstrumentedLock.java (add) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/InstrumentedLock.java (add) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/InstrumentedReadLock.java (add) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/InstrumentedReadWriteLock.java (add) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestInstrumentedReadWriteLock.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java (delete) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/InstrumentedLock.java (add) hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestInstrumentedLock.java
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Jingcheng Du for the contribution and Chris Douglas for the review/commit.
          Looking forward to the continued work in HDFS-9668.

          Show
          xiaochen Xiao Chen added a comment - Thanks Jingcheng Du for the contribution and Chris Douglas for the review/commit. Looking forward to the continued work in HDFS-9668 .

            People

            • Assignee:
              jingcheng.du@intel.com Jingcheng Du
              Reporter:
              jingcheng.du@intel.com Jingcheng Du
            • Votes:
              0 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development