Details

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

      Description

      This JIRA proposes to measure the time the of lock of FsDatasetImpl is held by a thread. Doing so will allow us to measure lock statistics.

      This can be done by extending the AutoCloseableLock lock object in FsDatasetImpl. In the future we can also consider replacing the lock with a read-write lock.

      1. HDFS-10742.001.patch
        13 kB
        Chen Liang
      2. HDFS-10742.002.patch
        11 kB
        Chen Liang
      3. HDFS-10742.003.patch
        11 kB
        Chen Liang
      4. HDFS-10742.004.patch
        10 kB
        Chen Liang
      5. HDFS-10742.005.patch
        10 kB
        Chen Liang
      6. HDFS-10742.006.patch
        10 kB
        Chen Liang
      7. HDFS-10742.007.patch
        10 kB
        Chen Liang
      8. HDFS-10742.008.patch
        10 kB
        Chen Liang
      9. HDFS-10742.009.patch
        6 kB
        Chen Liang
      10. HDFS-10742.010.patch
        6 kB
        Chen Liang
      11. HDFS-10742.011.patch
        11 kB
        Chen Liang
      12. HDFS-10742.012.patch
        11 kB
        Chen Liang
      13. HDFS-10742.013.patch
        11 kB
        Chen Liang
      14. HDFS-10742.014.patch
        11 kB
        Chen Liang
      15. HDFS-10742.015.patch
        16 kB
        Chris Douglas
      16. HDFS-10742.016.patch
        17 kB
        Chris Douglas
      17. HDFS-10742.017.patch
        18 kB
        Chris Douglas

        Issue Links

          Activity

          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 1 new or modified test files.
          0 mvndep 0m 25s Maven dependency ordering for branch
          +1 mvninstall 7m 14s trunk passed
          +1 compile 7m 2s trunk passed
          +1 checkstyle 1m 26s trunk passed
          +1 mvnsite 1m 49s trunk passed
          +1 mvneclipse 0m 25s trunk passed
          +1 findbugs 3m 3s trunk passed
          +1 javadoc 1m 46s trunk passed
          0 mvndep 0m 15s Maven dependency ordering for patch
          +1 mvninstall 1m 33s the patch passed
          +1 compile 7m 37s the patch passed
          +1 javac 7m 37s the patch passed
          -0 checkstyle 1m 28s root: The patch generated 1 new + 137 unchanged - 0 fixed = 138 total (was 137)
          +1 mvnsite 1m 51s the patch passed
          +1 mvneclipse 0m 26s the patch passed
          -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply
          +1 findbugs 3m 33s the patch passed
          +1 javadoc 1m 42s the patch passed
          -1 unit 19m 46s hadoop-common in the patch failed.
          -1 unit 86m 54s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 29s The patch does not generate ASF License warnings.
          149m 52s



          Reason Tests
          Failed junit tests hadoop.security.TestGroupsCaching
            hadoop.hdfs.security.TestDelegationTokenForProxyUser
          Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12822891/HDFS-10742.001.patch
          JIRA Issue HDFS-10742
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 28184fae56fe 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 / 85422bb
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16369/artifact/patchprocess/diff-checkstyle-root.txt
          whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/16369/artifact/patchprocess/whitespace-eol.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16369/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16369/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16369/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16369/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 1 new or modified test files. 0 mvndep 0m 25s Maven dependency ordering for branch +1 mvninstall 7m 14s trunk passed +1 compile 7m 2s trunk passed +1 checkstyle 1m 26s trunk passed +1 mvnsite 1m 49s trunk passed +1 mvneclipse 0m 25s trunk passed +1 findbugs 3m 3s trunk passed +1 javadoc 1m 46s trunk passed 0 mvndep 0m 15s Maven dependency ordering for patch +1 mvninstall 1m 33s the patch passed +1 compile 7m 37s the patch passed +1 javac 7m 37s the patch passed -0 checkstyle 1m 28s root: The patch generated 1 new + 137 unchanged - 0 fixed = 138 total (was 137) +1 mvnsite 1m 51s the patch passed +1 mvneclipse 0m 26s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix <<patch_file>>. Refer https://git-scm.com/docs/git-apply +1 findbugs 3m 33s the patch passed +1 javadoc 1m 42s the patch passed -1 unit 19m 46s hadoop-common in the patch failed. -1 unit 86m 54s hadoop-hdfs in the patch failed. +1 asflicense 0m 29s The patch does not generate ASF License warnings. 149m 52s Reason Tests Failed junit tests hadoop.security.TestGroupsCaching   hadoop.hdfs.security.TestDelegationTokenForProxyUser Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12822891/HDFS-10742.001.patch JIRA Issue HDFS-10742 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 28184fae56fe 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 / 85422bb Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16369/artifact/patchprocess/diff-checkstyle-root.txt whitespace https://builds.apache.org/job/PreCommit-HDFS-Build/16369/artifact/patchprocess/whitespace-eol.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/16369/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/16369/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16369/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16369/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          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 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 8m 3s trunk passed
          +1 compile 0m 56s trunk passed
          +1 checkstyle 0m 29s trunk passed
          +1 mvnsite 1m 1s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 53s trunk passed
          +1 javadoc 1m 1s trunk passed
          +1 mvninstall 0m 57s the patch passed
          +1 compile 0m 49s the patch passed
          +1 javac 0m 49s the patch passed
          +1 checkstyle 0m 27s the patch passed
          +1 mvnsite 0m 58s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 0s the patch passed
          +1 javadoc 0m 57s the patch passed
          -1 unit 62m 4s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          83m 51s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12822912/HDFS-10742.003.patch
          JIRA Issue HDFS-10742
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f25f05411bc2 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 / 9c6a438
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16372/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16372/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16372/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 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 8m 3s trunk passed +1 compile 0m 56s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 1m 1s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 53s trunk passed +1 javadoc 1m 1s trunk passed +1 mvninstall 0m 57s the patch passed +1 compile 0m 49s the patch passed +1 javac 0m 49s the patch passed +1 checkstyle 0m 27s the patch passed +1 mvnsite 0m 58s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 0s the patch passed +1 javadoc 0m 57s the patch passed -1 unit 62m 4s hadoop-hdfs in the patch failed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 83m 51s Reason Tests Failed junit tests hadoop.hdfs.server.balancer.TestBalancerWithMultipleNameNodes Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12822912/HDFS-10742.003.patch JIRA Issue HDFS-10742 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f25f05411bc2 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 / 9c6a438 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/16372/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16372/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16372/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          vagarychen Chen Liang added a comment -

          The failed test is unrelated.

          Show
          vagarychen Chen Liang added a comment - The failed test is unrelated.
          Hide
          chris.douglas Chris Douglas added a comment - - edited
          • Is this the intended ConcurrentHashMap implementation?
            +import org.jboss.netty.util.internal.ConcurrentHashMap;
            

            The patch synchronizes on the instance, rather than using ConcurrentMap features. The tracking code could be simplified using those methods.

          • Some commented code (e.g., + //Thread.dumpStack();)
          • Instead of adding the (unused) delay() method, consider passing in a org.apache.hadoop.util.Timer instance. This would also make it easier to write the unit test.
          • There's a comment explaining the importance of using thread-local storage, but the current patch neither uses nor requires it.
          • Please use the slf4j logger and vararg syntax instead of building strings that might be discarded/suppressed.
          • If the logger is null, shouldn't this fail? The Optional wrapper seems unnecessary.

          On a related note, the virtues of AutoCloseableLock (HADOOP-13466) are not obvious to me. Static analysis tools already catch what it enforces with AutoCloseable. Its interface is almost identical to java.util.concurrent.locks.Lock (admitting extensions like this). It seems to add an idiosyncrasy where there is already an idiom (with supporting tooling). Is there another advantage?

          Show
          chris.douglas Chris Douglas added a comment - - edited Is this the intended ConcurrentHashMap implementation? +import org.jboss.netty.util.internal.ConcurrentHashMap; The patch synchronizes on the instance, rather than using ConcurrentMap features. The tracking code could be simplified using those methods. Some commented code (e.g., + //Thread.dumpStack(); ) Instead of adding the (unused) delay() method, consider passing in a org.apache.hadoop.util.Timer instance. This would also make it easier to write the unit test. There's a comment explaining the importance of using thread-local storage, but the current patch neither uses nor requires it. Please use the slf4j logger and vararg syntax instead of building strings that might be discarded/suppressed. If the logger is null, shouldn't this fail? The Optional wrapper seems unnecessary. On a related note, the virtues of AutoCloseableLock ( HADOOP-13466 ) are not obvious to me. Static analysis tools already catch what it enforces with AutoCloseable . Its interface is almost identical to java.util.concurrent.locks.Lock (admitting extensions like this). It seems to add an idiosyncrasy where there is already an idiom (with supporting tooling). Is there another advantage?
          Hide
          vagarychen Chen Liang added a comment -

          Thanks for the catch Chris Douglas! Updated a patch that addresses most of the issues. (Changing to slf4j logger is a bit tricky as I'm directly using the log from FsDatasetImpl here).

          Regarding you note, the intentions of AutoCloseable was mainly to allow easy instrumentation such as the instrumented lock in this JIRA. I'm actually not quit familiar with the static analysis tools as you mentioned. Anything to add Arpit Agarwal?

          Show
          vagarychen Chen Liang added a comment - Thanks for the catch Chris Douglas ! Updated a patch that addresses most of the issues. (Changing to slf4j logger is a bit tricky as I'm directly using the log from FsDatasetImpl here). Regarding you note, the intentions of AutoCloseable was mainly to allow easy instrumentation such as the instrumented lock in this JIRA. I'm actually not quit familiar with the static analysis tools as you mentioned. Anything to add Arpit Agarwal ?
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          On a related note, the virtues of AutoCloseableLock (HADOOP-13466) are not obvious to me. Static analysis tools already catch what it enforces with AutoCloseable. Its interface is almost identical to java.util.concurrent.locks.Lock (admitting extensions like this). It seems to add an idiosyncrasy where there is already an idiom (with supporting tooling). Is there another advantage?

          Hi Chris, there is no other advantage. It just eliminates the finally block.

          Chen Liang, I'll take a look at your patch.

          Show
          arpitagarwal Arpit Agarwal added a comment - On a related note, the virtues of AutoCloseableLock ( HADOOP-13466 ) are not obvious to me. Static analysis tools already catch what it enforces with AutoCloseable. Its interface is almost identical to java.util.concurrent.locks.Lock (admitting extensions like this). It seems to add an idiosyncrasy where there is already an idiom (with supporting tooling). Is there another advantage? Hi Chris, there is no other advantage. It just eliminates the finally block. Chen Liang , I'll take a look at your patch.
          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 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 14s trunk passed
          +1 compile 0m 51s trunk passed
          +1 checkstyle 0m 27s trunk passed
          +1 mvnsite 0m 54s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 45s trunk passed
          +1 javadoc 0m 55s trunk passed
          +1 mvninstall 0m 48s the patch passed
          +1 compile 0m 46s the patch passed
          +1 javac 0m 46s the patch passed
          -0 checkstyle 0m 25s hadoop-hdfs-project/hadoop-hdfs: The patch generated 2 new + 109 unchanged - 0 fixed = 111 total (was 109)
          +1 mvnsite 0m 52s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 54s hadoop-hdfs-project/hadoop-hdfs generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)
          +1 javadoc 0m 52s the patch passed
          -1 unit 60m 29s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 17s The patch does not generate ASF License warnings.
          80m 23s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs
            Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:[line 164]
            Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:[line 180]
          Failed junit tests hadoop.hdfs.server.namenode.TestDecommissioningStatus



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825168/HDFS-10742.004.patch
          JIRA Issue HDFS-10742
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 29af66f6d836 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 / c37346d
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16520/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16520/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16520/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16520/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16520/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 12s 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 14s trunk passed +1 compile 0m 51s trunk passed +1 checkstyle 0m 27s trunk passed +1 mvnsite 0m 54s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 45s trunk passed +1 javadoc 0m 55s trunk passed +1 mvninstall 0m 48s the patch passed +1 compile 0m 46s the patch passed +1 javac 0m 46s the patch passed -0 checkstyle 0m 25s hadoop-hdfs-project/hadoop-hdfs: The patch generated 2 new + 109 unchanged - 0 fixed = 111 total (was 109) +1 mvnsite 0m 52s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 54s hadoop-hdfs-project/hadoop-hdfs generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) +1 javadoc 0m 52s the patch passed -1 unit 60m 29s hadoop-hdfs in the patch failed. +1 asflicense 0m 17s The patch does not generate ASF License warnings. 80m 23s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs   Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java: [line 164]   Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java: [line 180] Failed junit tests hadoop.hdfs.server.namenode.TestDecommissioningStatus Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825168/HDFS-10742.004.patch JIRA Issue HDFS-10742 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 29af66f6d836 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 / c37346d Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16520/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16520/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/16520/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16520/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16520/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          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 49s trunk passed
          +1 compile 0m 44s trunk passed
          +1 checkstyle 0m 25s trunk passed
          +1 mvnsite 0m 50s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 42s trunk passed
          +1 javadoc 0m 55s trunk passed
          +1 mvninstall 0m 45s the patch passed
          +1 compile 0m 41s the patch passed
          +1 javac 0m 41s the patch passed
          -0 checkstyle 0m 23s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 109 unchanged - 0 fixed = 110 total (was 109)
          +1 mvnsite 0m 46s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 45s hadoop-hdfs-project/hadoop-hdfs generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)
          +1 javadoc 0m 54s the patch passed
          +1 unit 75m 23s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          94m 3s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs
            Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:[line 163]
            Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:[line 179]



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825170/HDFS-10742.004.patch
          JIRA Issue HDFS-10742
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 76b0721701a5 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 / c37346d
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16521/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16521/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16521/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16521/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 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 49s trunk passed +1 compile 0m 44s trunk passed +1 checkstyle 0m 25s trunk passed +1 mvnsite 0m 50s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 42s trunk passed +1 javadoc 0m 55s trunk passed +1 mvninstall 0m 45s the patch passed +1 compile 0m 41s the patch passed +1 javac 0m 41s the patch passed -0 checkstyle 0m 23s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 109 unchanged - 0 fixed = 110 total (was 109) +1 mvnsite 0m 46s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 45s hadoop-hdfs-project/hadoop-hdfs generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) +1 javadoc 0m 54s the patch passed +1 unit 75m 23s hadoop-hdfs in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 94m 3s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs   Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java: [line 163]   Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java: [line 179] Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825170/HDFS-10742.004.patch JIRA Issue HDFS-10742 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 76b0721701a5 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 / c37346d Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16521/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16521/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16521/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16521/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          vagarychen Chen Liang added a comment -

          Thanks Arpit Agarwal!

          Updated a patch to fix style. findbugs reports a potentially not atomic operation. But this is actually the desired behavior and it does not need to atomic. (Also, earlier patches had the same code and for some reason findbugs did not complain).

          Show
          vagarychen Chen Liang added a comment - Thanks Arpit Agarwal ! Updated a patch to fix style. findbugs reports a potentially not atomic operation. But this is actually the desired behavior and it does not need to atomic. (Also, earlier patches had the same code and for some reason findbugs did not complain).
          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 42s trunk passed
          +1 compile 0m 44s trunk passed
          +1 checkstyle 0m 26s trunk passed
          +1 mvnsite 0m 51s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 41s trunk passed
          +1 javadoc 0m 54s trunk passed
          +1 mvninstall 0m 46s the patch passed
          +1 compile 0m 42s the patch passed
          +1 javac 0m 42s the patch passed
          +1 checkstyle 0m 24s the patch passed
          +1 mvnsite 0m 48s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 47s hadoop-hdfs-project/hadoop-hdfs generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)
          +1 javadoc 0m 52s the patch passed
          +1 unit 58m 8s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          76m 49s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs
            Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:[line 163]
            Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:[line 179]



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825294/HDFS-10742.005.patch
          JIRA Issue HDFS-10742
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux be3739531a9a 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 / 6fa9bf4
          Default Java 1.8.0_101
          findbugs v3.0.0
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16527/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16527/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16527/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 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 42s trunk passed +1 compile 0m 44s trunk passed +1 checkstyle 0m 26s trunk passed +1 mvnsite 0m 51s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 41s trunk passed +1 javadoc 0m 54s trunk passed +1 mvninstall 0m 46s the patch passed +1 compile 0m 42s the patch passed +1 javac 0m 42s the patch passed +1 checkstyle 0m 24s the patch passed +1 mvnsite 0m 48s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 47s hadoop-hdfs-project/hadoop-hdfs generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) +1 javadoc 0m 52s the patch passed +1 unit 58m 8s hadoop-hdfs in the patch passed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 76m 49s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs   Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java: [line 163]   Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java: [line 179] Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825294/HDFS-10742.005.patch JIRA Issue HDFS-10742 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux be3739531a9a 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 / 6fa9bf4 Default Java 1.8.0_101 findbugs v3.0.0 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16527/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16527/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16527/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          vagarychen Chen Liang added a comment -

          The findbugs still complains the issue, slightly modified the code to address it.

          Show
          vagarychen Chen Liang added a comment - The findbugs still complains the issue, slightly modified the code to address it.
          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 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 9m 27s trunk passed
          +1 compile 0m 58s trunk passed
          +1 checkstyle 0m 31s trunk passed
          +1 mvnsite 1m 10s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 2m 8s trunk passed
          +1 javadoc 0m 55s trunk passed
          +1 mvninstall 0m 47s the patch passed
          +1 compile 0m 43s the patch passed
          +1 javac 0m 43s the patch passed
          +1 checkstyle 0m 25s the patch passed
          +1 mvnsite 0m 54s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 50s hadoop-hdfs-project/hadoop-hdfs generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0)
          +1 javadoc 0m 54s the patch passed
          -1 unit 77m 35s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          100m 41s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs
            Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:[line 162]
            Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:[line 175]
          Failed junit tests hadoop.hdfs.server.namenode.snapshot.TestSnapshotFileLength



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825311/HDFS-10742.006.patch
          JIRA Issue HDFS-10742
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ec48519e1d32 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 / 3476156
          Default Java 1.8.0_101
          findbugs v3.0.0
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16528/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16528/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16528/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16528/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 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 9m 27s trunk passed +1 compile 0m 58s trunk passed +1 checkstyle 0m 31s trunk passed +1 mvnsite 1m 10s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 2m 8s trunk passed +1 javadoc 0m 55s trunk passed +1 mvninstall 0m 47s the patch passed +1 compile 0m 43s the patch passed +1 javac 0m 43s the patch passed +1 checkstyle 0m 25s the patch passed +1 mvnsite 0m 54s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 50s hadoop-hdfs-project/hadoop-hdfs generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) +1 javadoc 0m 54s the patch passed -1 unit 77m 35s hadoop-hdfs in the patch failed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 100m 41s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs   Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java: [line 162]   Sequence of calls to java.util.concurrent.ConcurrentHashMap may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:may not be atomic in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java: [line 175] Failed junit tests hadoop.hdfs.server.namenode.snapshot.TestSnapshotFileLength Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825311/HDFS-10742.006.patch JIRA Issue HDFS-10742 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ec48519e1d32 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 / 3476156 Default Java 1.8.0_101 findbugs v3.0.0 findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16528/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/16528/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16528/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16528/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 -

          there is no other advantage. It just eliminates the finally block

          Since Hadoop already uses Lock (including the capabilities omitted from AutoCloseableLock), it's unlikely we'd standardize on this abstraction. It adds a special case for a very common pattern to a subset of the code, for some mild syntactic sugar.

          • This should have a unit test.
          • The local* variables in check appear to be intended for volatile variables, but the fields are not
          • Please fix the findbugs warnings by either using ConcurrentMap as designed or using non-threadsafe Map classes and the existing synchronization (though using get/null checks, rather than containsKey).
          • If this can't use slf4j, then consider using isWarnEnabled() to guard the string formatting.and/or ignore all of check
          • Consider passing the logging thresholds as parameters, rather than hard-coding them in the class
          Show
          chris.douglas Chris Douglas added a comment - there is no other advantage. It just eliminates the finally block Since Hadoop already uses Lock (including the capabilities omitted from AutoCloseableLock ), it's unlikely we'd standardize on this abstraction. It adds a special case for a very common pattern to a subset of the code, for some mild syntactic sugar. This should have a unit test. The local* variables in check appear to be intended for volatile variables, but the fields are not Please fix the findbugs warnings by either using ConcurrentMap as designed or using non-threadsafe Map classes and the existing synchronization (though using get /null checks, rather than containsKey ). If this can't use slf4j, then consider using isWarnEnabled() to guard the string formatting.and/or ignore all of check Consider passing the logging thresholds as parameters, rather than hard-coding them in the class
          Hide
          vagarychen Chen Liang added a comment -

          Thanks Chris Douglas for the comments!

          Updated a patch to fix Jenkins findbugs complains. Also reverted an unnecessary change from previous patch that could be misleading. Will update with unit test later on.

          Apart from this. I'm considering using hadoop's built-in metric system, instead of maintaining and printing locally maintained stats, what do you think about this?

          Show
          vagarychen Chen Liang added a comment - Thanks Chris Douglas for the comments! Updated a patch to fix Jenkins findbugs complains. Also reverted an unnecessary change from previous patch that could be misleading. Will update with unit test later on. Apart from this. I'm considering using hadoop's built-in metric system, instead of maintaining and printing locally maintained stats, what do you think about this?
          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 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 18s trunk passed
          +1 compile 0m 47s trunk passed
          +1 checkstyle 0m 26s trunk passed
          +1 mvnsite 0m 53s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 44s trunk passed
          +1 javadoc 0m 56s trunk passed
          +1 mvninstall 0m 46s the patch passed
          +1 compile 0m 43s the patch passed
          +1 javac 0m 43s the patch passed
          -0 checkstyle 0m 24s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 109 unchanged - 0 fixed = 113 total (was 109)
          +1 mvnsite 0m 49s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 52s the patch passed
          +1 javadoc 0m 51s the patch passed
          -1 unit 58m 0s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          77m 38s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.namenode.ha.TestHASafeMode
            hadoop.hdfs.server.namenode.TestEditLogJournalFailures



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10742
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825507/HDFS-10742.007.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux c2ecb97868a7 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 / 1360bd2
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16539/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16539/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16539/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16539/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 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 18s trunk passed +1 compile 0m 47s trunk passed +1 checkstyle 0m 26s trunk passed +1 mvnsite 0m 53s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 44s trunk passed +1 javadoc 0m 56s trunk passed +1 mvninstall 0m 46s the patch passed +1 compile 0m 43s the patch passed +1 javac 0m 43s the patch passed -0 checkstyle 0m 24s hadoop-hdfs-project/hadoop-hdfs: The patch generated 4 new + 109 unchanged - 0 fixed = 113 total (was 109) +1 mvnsite 0m 49s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 52s the patch passed +1 javadoc 0m 51s the patch passed -1 unit 58m 0s hadoop-hdfs in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 77m 38s Reason Tests Failed junit tests hadoop.hdfs.server.namenode.ha.TestHASafeMode   hadoop.hdfs.server.namenode.TestEditLogJournalFailures Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10742 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825507/HDFS-10742.007.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux c2ecb97868a7 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 / 1360bd2 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16539/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/16539/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16539/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16539/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          vagarychen Chen Liang added a comment -

          fix checkstyle

          Show
          vagarychen Chen Liang added a comment - fix checkstyle
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 24s 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 41s trunk passed
          +1 compile 0m 50s trunk passed
          +1 checkstyle 0m 31s trunk passed
          +1 mvnsite 0m 56s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 45s trunk passed
          +1 javadoc 1m 3s trunk passed
          +1 mvninstall 0m 53s the patch passed
          +1 compile 0m 45s the patch passed
          +1 javac 0m 45s the patch passed
          +1 checkstyle 0m 25s the patch passed
          +1 mvnsite 0m 50s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 48s the patch passed
          +1 javadoc 0m 58s the patch passed
          -1 unit 76m 13s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          97m 2s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.blockmanagement.TestPendingInvalidateBlock
            hadoop.hdfs.server.namenode.TestEditLogJournalFailures



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10742
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825515/HDFS-10742.008.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ab90c40c203a 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 / 1360bd2
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16542/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16542/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16542/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 24s 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 41s trunk passed +1 compile 0m 50s trunk passed +1 checkstyle 0m 31s trunk passed +1 mvnsite 0m 56s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 45s trunk passed +1 javadoc 1m 3s trunk passed +1 mvninstall 0m 53s the patch passed +1 compile 0m 45s the patch passed +1 javac 0m 45s the patch passed +1 checkstyle 0m 25s the patch passed +1 mvnsite 0m 50s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 48s the patch passed +1 javadoc 0m 58s the patch passed -1 unit 76m 13s hadoop-hdfs in the patch failed. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 97m 2s Reason Tests Failed junit tests hadoop.hdfs.server.blockmanagement.TestPendingInvalidateBlock   hadoop.hdfs.server.namenode.TestEditLogJournalFailures Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10742 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12825515/HDFS-10742.008.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ab90c40c203a 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 / 1360bd2 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/16542/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16542/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16542/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          arpitagarwal Arpit Agarwal added a comment - - edited

          Since Hadoop already uses Lock (including the capabilities omitted from AutoCloseableLock), it's unlikely we'd standardize on this abstraction.

          Hi Chris, you may be right. I can't be sure other developers will prefer using this instead of manually adding the unlock calls. findbugs doesn't eliminate human error as committers can miss findbugs warnings (I've been guilty of this too).

          Show
          arpitagarwal Arpit Agarwal added a comment - - edited Since Hadoop already uses Lock (including the capabilities omitted from AutoCloseableLock), it's unlikely we'd standardize on this abstraction. Hi Chris, you may be right. I can't be sure other developers will prefer using this instead of manually adding the unlock calls. findbugs doesn't eliminate human error as committers can miss findbugs warnings (I've been guilty of this too).
          Hide
          chris.douglas Chris Douglas added a comment -

          I can't be sure other developers will prefer using this instead of manually adding the unlock calls. findbugs doesn't eliminate human error as committers can miss findbugs warnings (I've been guilty of this too).

          It's not just Hadoop that's standardized on Lock, it's Java. Moreover, AutoCloseableLock doesn't actually restrict the API to make it less error-prone.

          If its sole purpose is this syntactic sugar, perhaps this class is sufficient:

          public final class AutoLock implements Closeable {
            private final Lock lock;
            public AutoLock(Lock lock) {
              lock.lock();
              this.lock = lock;
            }
            public AutoLock(Lock lock, long timeout, TimeUnit unit) throws TimeoutException {
              if (!lock.tryLock(timeout, unit)) {
                throw new TimeoutException();
              }
              this.lock = lock;
            }
            @Override
            public void close() {
              lock.unlock();
            }
          }  
          

          Then this JIRA (and variants) could remain behind the Lock interface, rather than a custom API.

          I'm considering using hadoop's built-in metric system, instead of maintaining and printing locally maintained stats, what do you think about this?

          That's less information than this records. Is the intent to (dis)prove a hypothesis about FsDatasetImpl as a bottleneck, or to debug a common, known issue with stack traces, etc.?

          Show
          chris.douglas Chris Douglas added a comment - I can't be sure other developers will prefer using this instead of manually adding the unlock calls. findbugs doesn't eliminate human error as committers can miss findbugs warnings (I've been guilty of this too). It's not just Hadoop that's standardized on Lock , it's Java. Moreover, AutoCloseableLock doesn't actually restrict the API to make it less error-prone. If its sole purpose is this syntactic sugar, perhaps this class is sufficient: public final class AutoLock implements Closeable { private final Lock lock; public AutoLock(Lock lock) { lock.lock(); this .lock = lock; } public AutoLock(Lock lock, long timeout, TimeUnit unit) throws TimeoutException { if (!lock.tryLock(timeout, unit)) { throw new TimeoutException(); } this .lock = lock; } @Override public void close() { lock.unlock(); } } Then this JIRA (and variants) could remain behind the Lock interface, rather than a custom API. I'm considering using hadoop's built-in metric system, instead of maintaining and printing locally maintained stats, what do you think about this? That's less information than this records. Is the intent to (dis)prove a hypothesis about FsDatasetImpl as a bottleneck, or to debug a common, known issue with stack traces, etc.?
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          IIUC that'd be used like

          try (AutoCloseable ac = new AutoLock(lock)) {
          ...
          }
          

          That adds a new object allocation for every lock acquire so it's probably not very GC-friendly.

          Show
          arpitagarwal Arpit Agarwal added a comment - IIUC that'd be used like try (AutoCloseable ac = new AutoLock(lock)) { ... } That adds a new object allocation for every lock acquire so it's probably not very GC-friendly.
          Hide
          chris.douglas Chris Douglas added a comment -

          That adds a new object allocation for every lock acquire so it's probably not very GC-friendly.

          How so? If it's not inlined, the allocation is likely thread-local. Even if neither is true, the allocation should be GC-friendly by being short-lived (the critical section should be short). I could make up some nonsense about the dispatch overhead for each call, or the cost of loading the class, but in the context of a patch generating thread stacks and maintaining persistent maps of timestamps, it's absurd to debate any of this.

          Show
          chris.douglas Chris Douglas added a comment - That adds a new object allocation for every lock acquire so it's probably not very GC-friendly. How so? If it's not inlined, the allocation is likely thread-local. Even if neither is true, the allocation should be GC-friendly by being short-lived (the critical section should be short). I could make up some nonsense about the dispatch overhead for each call, or the cost of loading the class, but in the context of a patch generating thread stacks and maintaining persistent maps of timestamps, it's absurd to debate any of this.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Hi Chen Liang, a couple of comments:

          1. check() should have minimal overhead if the lock was not held for more lockWarningThresholdMs which will be the common case. We can simplify the patch quite a bit and just add logging along with the caller stack trace if the lock was held for too long. We can extend it later if we want to add performance statistics but we'd have to do it in a way that avoids object allocations.
          2. InstrumentedLock should probably not extend the concrete class AutoCloseableLock. We can make InstrumentedLock a separate class. or alternatively AutoCloseableLock a Java interface with multiple implementations.

          but in the context of a patch generating thread stacks and maintaining persistent maps of timestamps, it's absurd to debate any of this.

          Yes I agree. We should avoid that overhead.

          Show
          arpitagarwal Arpit Agarwal added a comment - Hi Chen Liang , a couple of comments: check() should have minimal overhead if the lock was not held for more lockWarningThresholdMs which will be the common case. We can simplify the patch quite a bit and just add logging along with the caller stack trace if the lock was held for too long. We can extend it later if we want to add performance statistics but we'd have to do it in a way that avoids object allocations. InstrumentedLock should probably not extend the concrete class AutoCloseableLock. We can make InstrumentedLock a separate class. or alternatively AutoCloseableLock a Java interface with multiple implementations. but in the context of a patch generating thread stacks and maintaining persistent maps of timestamps, it's absurd to debate any of this. Yes I agree. We should avoid that overhead.
          Hide
          vagarychen Chen Liang added a comment -

          Removed per-method measurement and aggregate stats to minimize overhead for cheap operations

          Show
          vagarychen Chen Liang added a comment - Removed per-method measurement and aggregate stats to minimize overhead for cheap operations
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s 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 41s trunk passed
          +1 compile 0m 49s trunk passed
          +1 checkstyle 0m 28s trunk passed
          +1 mvnsite 1m 1s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 56s trunk passed
          +1 javadoc 1m 2s trunk passed
          +1 mvninstall 0m 52s the patch passed
          +1 compile 0m 45s the patch passed
          +1 javac 0m 45s the patch passed
          -0 checkstyle 0m 25s hadoop-hdfs-project/hadoop-hdfs: The patch generated 9 new + 109 unchanged - 0 fixed = 118 total (was 109)
          +1 mvnsite 0m 50s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 54s the patch passed
          +1 javadoc 0m 56s the patch passed
          -1 unit 61m 58s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          82m 59s



          Reason Tests
          Failed junit tests hadoop.hdfs.TestRollingUpgrade



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10742
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826286/HDFS-10742.009.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 741e0e8df98c 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 / d6d9cff
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16582/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16582/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16582/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16582/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 21s 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 41s trunk passed +1 compile 0m 49s trunk passed +1 checkstyle 0m 28s trunk passed +1 mvnsite 1m 1s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 56s trunk passed +1 javadoc 1m 2s trunk passed +1 mvninstall 0m 52s the patch passed +1 compile 0m 45s the patch passed +1 javac 0m 45s the patch passed -0 checkstyle 0m 25s hadoop-hdfs-project/hadoop-hdfs: The patch generated 9 new + 109 unchanged - 0 fixed = 118 total (was 109) +1 mvnsite 0m 50s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 54s the patch passed +1 javadoc 0m 56s the patch passed -1 unit 61m 58s hadoop-hdfs in the patch failed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 82m 59s Reason Tests Failed junit tests hadoop.hdfs.TestRollingUpgrade Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10742 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826286/HDFS-10742.009.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 741e0e8df98c 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 / d6d9cff Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16582/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/16582/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16582/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16582/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          vagarychen Chen Liang added a comment -

          fix checkstyle

          Show
          vagarychen Chen Liang added a comment - fix checkstyle
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Thanks Chen Liang. The change lgtm. A few minor comments:

          1. The InstrumentedLock class javadoc refers to LockChecker.
          2. Missing Javadoc for InstrumentedLock constructor.
          3. We should add a unit test for InstrumentedLock, similar to the one for AutoCloseableLock.

          Couple of comments that can be handled in a separate Jira:

          1. InstrumentedLock should not extend the concrete class AutoCloseableLock. We could make AutoCloseableLock an interface.
          2. The thresholds in FsDatasetImpl constructor should probably be configurable. We could use the same config key being introduced by HDFS-10713.
          Show
          arpitagarwal Arpit Agarwal added a comment - Thanks Chen Liang . The change lgtm. A few minor comments: The InstrumentedLock class javadoc refers to LockChecker. Missing Javadoc for InstrumentedLock constructor. We should add a unit test for InstrumentedLock, similar to the one for AutoCloseableLock. Couple of comments that can be handled in a separate Jira: InstrumentedLock should not extend the concrete class AutoCloseableLock. We could make AutoCloseableLock an interface. The thresholds in FsDatasetImpl constructor should probably be configurable. We could use the same config key being introduced by HDFS-10713 .
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 26s 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 8m 24s trunk passed
          +1 compile 0m 57s trunk passed
          +1 checkstyle 0m 29s trunk passed
          +1 mvnsite 1m 1s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 51s trunk passed
          +1 javadoc 1m 0s trunk passed
          +1 mvninstall 0m 58s the patch passed
          +1 compile 0m 52s the patch passed
          +1 javac 0m 52s the patch passed
          +1 checkstyle 0m 26s the patch passed
          +1 mvnsite 0m 59s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 2m 3s the patch passed
          +1 javadoc 0m 58s the patch passed
          +1 unit 86m 37s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          109m 7s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10742
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826457/HDFS-10742.010.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 25eb3cb7c6e7 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 / 20ae1fa
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16593/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16593/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 26s 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 8m 24s trunk passed +1 compile 0m 57s trunk passed +1 checkstyle 0m 29s trunk passed +1 mvnsite 1m 1s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 51s trunk passed +1 javadoc 1m 0s trunk passed +1 mvninstall 0m 58s the patch passed +1 compile 0m 52s the patch passed +1 javac 0m 52s the patch passed +1 checkstyle 0m 26s the patch passed +1 mvnsite 0m 59s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 2m 3s the patch passed +1 javadoc 0m 58s the patch passed +1 unit 86m 37s hadoop-hdfs in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 109m 7s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10742 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12826457/HDFS-10742.010.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 25eb3cb7c6e7 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 / 20ae1fa Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16593/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16593/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          vagarychen Chen Liang added a comment -

          Thanks Arpit Agarwal for the review! Updated a patch to reflect these changes.

          Show
          vagarychen Chen Liang added a comment - Thanks Arpit Agarwal for the review! Updated a patch to reflect these changes.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Hi Chris Douglas, are you okay with proceeding? Was your concern was with exposing AutoCloseableLock in hadoop utils or just an overall objection to this idiom even if it's not exposed outside the DN.

          Show
          arpitagarwal Arpit Agarwal added a comment - Hi Chris Douglas , are you okay with proceeding? Was your concern was with exposing AutoCloseableLock in hadoop utils or just an overall objection to this idiom even if it's not exposed outside the DN.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          By the way I am +1 on the v11 patch. Will hold off committing for a bit until Chris gets a chance to comment.

          Show
          arpitagarwal Arpit Agarwal added a comment - By the way I am +1 on the v11 patch. Will hold off committing for a bit until Chris gets a chance to comment.
          Hide
          chris.douglas Chris Douglas added a comment -

          Removed per-method measurement and aggregate stats to minimize overhead for cheap operations

          Recording stack traces, etc. may be useful. I raised those overheads for two reasons. First, that diagnostic data should only be built when necessary. Previous versions of the patch generated diagnostic data that was ultimately discarded. Second, minutiae like "GC-friendliness", whether a wrapper class on Lock adds dispatch overhead, class loading overhead, etc. when this JIRA logs critical sections longer than 300ms are irrelevant, literally by orders of magnitude.

          I'll ask again after the motivation. Is the intent to identify subsystems that do too much work (and/or I/O) in a critical section? To measure where the DN is spending cycles? In some of these cases, stack traces could be very helpful. As long as this telemetry can be tuned by configuration, it shouldn't create new problems for operators. To that point, instead of reusing the FsDatasetImpl log, this may want to use its own (so disabling/redirecting it doesn't affect other, critical information written to that log).

          Was your concern was with exposing AutoCloseableLock in hadoop utils or just an overall objection to this idiom even if it's not exposed outside the DN.

          I have two objections, one to the idiom and one to the design.

          AutoCloseableLock is a new abstraction that doesn't elide any details of the thing it obscures. Every path for Lock objects is possible, so it doesn't make it easier to reason about lock correctness. It creates a special type for the simplest possible case. This case is so simple, we have automated tooling to detect it. Further, instead of every Java developer looking at a Lock and scanning for well-worn expectations, we introduce an abstraction that requires investigation. No advantage beyond developer convenience has been raised, and it is a demonstrably inconvenient abstraction.

          If the interface were limited so it could only be used in the resource idiom (as proposed earlier), then a developer could know that the Lock acquisition was limited to scope. As semantic sugar it would still be pointless, because Java programmers also know how to use synchronized.

          This could be redeemed by changing the design. If the AutoCloseableLock constructor accepted a Lock instance and cut its API to scope-limited changes, then telemetry like this could be added as implementations of the Lock type with all the perceived advantages of ACL. Devs can reason about the simpler API of ACL (scope-based acquisition), but still monitor all acquisitions as required by this JIRA (impractical to implement using synchronized).

          I remain skeptical of AutoCloseableLock, since one could just use the Lock in interface and existing tooling to implement the same. But if its virtues are obvious to everyone but me I won't stand in the way of including it.

          Show
          chris.douglas Chris Douglas added a comment - Removed per-method measurement and aggregate stats to minimize overhead for cheap operations Recording stack traces, etc. may be useful. I raised those overheads for two reasons. First, that diagnostic data should only be built when necessary. Previous versions of the patch generated diagnostic data that was ultimately discarded. Second, minutiae like "GC-friendliness", whether a wrapper class on Lock adds dispatch overhead, class loading overhead, etc. when this JIRA logs critical sections longer than 300ms are irrelevant, literally by orders of magnitude. I'll ask again after the motivation. Is the intent to identify subsystems that do too much work (and/or I/O) in a critical section? To measure where the DN is spending cycles? In some of these cases, stack traces could be very helpful. As long as this telemetry can be tuned by configuration, it shouldn't create new problems for operators. To that point, instead of reusing the FsDatasetImpl log, this may want to use its own (so disabling/redirecting it doesn't affect other, critical information written to that log). Was your concern was with exposing AutoCloseableLock in hadoop utils or just an overall objection to this idiom even if it's not exposed outside the DN. I have two objections, one to the idiom and one to the design. AutoCloseableLock is a new abstraction that doesn't elide any details of the thing it obscures. Every path for Lock objects is possible, so it doesn't make it easier to reason about lock correctness. It creates a special type for the simplest possible case. This case is so simple, we have automated tooling to detect it. Further, instead of every Java developer looking at a Lock and scanning for well-worn expectations, we introduce an abstraction that requires investigation. No advantage beyond developer convenience has been raised, and it is a demonstrably inconvenient abstraction. If the interface were limited so it could only be used in the resource idiom (as proposed earlier), then a developer could know that the Lock acquisition was limited to scope. As semantic sugar it would still be pointless, because Java programmers also know how to use synchronized . This could be redeemed by changing the design. If the AutoCloseableLock constructor accepted a Lock instance and cut its API to scope-limited changes, then telemetry like this could be added as implementations of the Lock type with all the perceived advantages of ACL. Devs can reason about the simpler API of ACL (scope-based acquisition), but still monitor all acquisitions as required by this JIRA (impractical to implement using synchronized ). I remain skeptical of AutoCloseableLock , since one could just use the Lock in interface and existing tooling to implement the same. But if its virtues are obvious to everyone but me I won't stand in the way of including it.
          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 1 new or modified test files.
          +1 mvninstall 6m 56s trunk passed
          +1 compile 0m 44s trunk passed
          +1 checkstyle 0m 26s trunk passed
          +1 mvnsite 0m 50s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 49s trunk passed
          +1 javadoc 0m 59s trunk passed
          +1 mvninstall 0m 56s the patch passed
          +1 compile 0m 53s the patch passed
          +1 javac 0m 53s the patch passed
          -0 checkstyle 0m 28s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 109 unchanged - 0 fixed = 110 total (was 109)
          +1 mvnsite 0m 57s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 2m 3s hadoop-hdfs-project/hadoop-hdfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0)
          +1 javadoc 0m 59s the patch passed
          -1 unit 65m 36s hadoop-hdfs in the patch failed.
          -1 asflicense 0m 19s The patch generated 1 ASF License warnings.
          85m 58s



          Reason Tests
          FindBugs module:hadoop-hdfs-project/hadoop-hdfs
            Format string should use %n rather than n in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:rather than n in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:[line 118]
          Failed junit tests hadoop.hdfs.server.namenode.TestListCorruptFileBlocks



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10742
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827246/HDFS-10742.012.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux cf8d9e71b675 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 / f6c0b75
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16651/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt
          findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16651/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16651/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16651/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/16651/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16651/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 1 new or modified test files. +1 mvninstall 6m 56s trunk passed +1 compile 0m 44s trunk passed +1 checkstyle 0m 26s trunk passed +1 mvnsite 0m 50s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 49s trunk passed +1 javadoc 0m 59s trunk passed +1 mvninstall 0m 56s the patch passed +1 compile 0m 53s the patch passed +1 javac 0m 53s the patch passed -0 checkstyle 0m 28s hadoop-hdfs-project/hadoop-hdfs: The patch generated 1 new + 109 unchanged - 0 fixed = 110 total (was 109) +1 mvnsite 0m 57s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 2m 3s hadoop-hdfs-project/hadoop-hdfs generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) +1 javadoc 0m 59s the patch passed -1 unit 65m 36s hadoop-hdfs in the patch failed. -1 asflicense 0m 19s The patch generated 1 ASF License warnings. 85m 58s Reason Tests FindBugs module:hadoop-hdfs-project/hadoop-hdfs   Format string should use %n rather than n in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java:rather than n in org.apache.hadoop.hdfs.InstrumentedLock.check(long, long) At InstrumentedLock.java: [line 118] Failed junit tests hadoop.hdfs.server.namenode.TestListCorruptFileBlocks Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10742 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827246/HDFS-10742.012.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux cf8d9e71b675 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 / f6c0b75 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/16651/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project_hadoop-hdfs.txt findbugs https://builds.apache.org/job/PreCommit-HDFS-Build/16651/artifact/patchprocess/new-findbugs-hadoop-hdfs-project_hadoop-hdfs.html unit https://builds.apache.org/job/PreCommit-HDFS-Build/16651/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16651/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/16651/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16651/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          vagarychen Chen Liang added a comment -

          fix checkstyle and findbug warnings

          Show
          vagarychen Chen Liang added a comment - fix checkstyle and findbug warnings
          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 1 new or modified test files.
          +1 mvninstall 6m 36s trunk passed
          +1 compile 0m 43s trunk passed
          +1 checkstyle 0m 26s trunk passed
          +1 mvnsite 0m 50s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 39s trunk passed
          +1 javadoc 0m 53s trunk passed
          +1 mvninstall 0m 45s the patch passed
          +1 compile 0m 41s the patch passed
          +1 javac 0m 41s the patch passed
          +1 checkstyle 0m 24s the patch passed
          +1 mvnsite 0m 48s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 45s the patch passed
          +1 javadoc 0m 51s the patch passed
          +1 unit 57m 48s hadoop-hdfs in the patch passed.
          -1 asflicense 0m 18s The patch generated 1 ASF License warnings.
          76m 11s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10742
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827260/HDFS-10742.013.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 6fdfe42601e7 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 / 5f23abf
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16653/testReport/
          asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/16653/artifact/patchprocess/patch-asflicense-problems.txt
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16653/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 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 1 new or modified test files. +1 mvninstall 6m 36s trunk passed +1 compile 0m 43s trunk passed +1 checkstyle 0m 26s trunk passed +1 mvnsite 0m 50s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 39s trunk passed +1 javadoc 0m 53s trunk passed +1 mvninstall 0m 45s the patch passed +1 compile 0m 41s the patch passed +1 javac 0m 41s the patch passed +1 checkstyle 0m 24s the patch passed +1 mvnsite 0m 48s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 45s the patch passed +1 javadoc 0m 51s the patch passed +1 unit 57m 48s hadoop-hdfs in the patch passed. -1 asflicense 0m 18s The patch generated 1 ASF License warnings. 76m 11s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10742 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827260/HDFS-10742.013.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 6fdfe42601e7 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 / 5f23abf Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16653/testReport/ asflicense https://builds.apache.org/job/PreCommit-HDFS-Build/16653/artifact/patchprocess/patch-asflicense-problems.txt modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16653/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          vagarychen Chen Liang added a comment -

          Somehow missed the license in previous patch, fixed it.

          Show
          vagarychen Chen Liang added a comment - Somehow missed the license in previous patch, fixed it.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 22s 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 27s trunk passed
          +1 compile 0m 48s trunk passed
          +1 checkstyle 0m 31s trunk passed
          +1 mvnsite 0m 54s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 45s trunk passed
          +1 javadoc 1m 3s trunk passed
          +1 mvninstall 0m 54s the patch passed
          +1 compile 0m 45s the patch passed
          +1 javac 0m 45s the patch passed
          +1 checkstyle 0m 24s the patch passed
          +1 mvnsite 0m 51s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 47s the patch passed
          +1 javadoc 0m 53s the patch passed
          +1 unit 73m 34s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          94m 3s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10742
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827273/HDFS-10742.014.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux a86b8660d560 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 / 5f23abf
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16654/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16654/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 22s 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 27s trunk passed +1 compile 0m 48s trunk passed +1 checkstyle 0m 31s trunk passed +1 mvnsite 0m 54s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 45s trunk passed +1 javadoc 1m 3s trunk passed +1 mvninstall 0m 54s the patch passed +1 compile 0m 45s the patch passed +1 javac 0m 45s the patch passed +1 checkstyle 0m 24s the patch passed +1 mvnsite 0m 51s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 47s the patch passed +1 javadoc 0m 53s the patch passed +1 unit 73m 34s hadoop-hdfs in the patch passed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 94m 3s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10742 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827273/HDFS-10742.014.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a86b8660d560 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 / 5f23abf Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16654/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project/hadoop-hdfs Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16654/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 -

          Modified v014 of the patch to show the alternative, since I could not convey the design in prose:

          • Changed AutoClosableLock and InstrumentedLock to wrap a Lock instance, rather than hard-coding ReentrantLock
          • Changed visibility of AutoCloseableLock#isLocked to package-private for unit tests, and annotated same
          • Changed time arithmetic to match guidance for System#nanotime
          • Changed init to not suppress the first lock warning if the first acquisition is within the log delay period
          • Changed logging check to avoid synchronizing on the instance
          • Changed the unit test to use Timer instead of Thread#sleep

          This retains most of the pattern as requested.

          Show
          chris.douglas Chris Douglas added a comment - Modified v014 of the patch to show the alternative, since I could not convey the design in prose: Changed AutoClosableLock and InstrumentedLock to wrap a Lock instance, rather than hard-coding ReentrantLock Changed visibility of AutoCloseableLock#isLocked to package-private for unit tests, and annotated same Changed time arithmetic to match guidance for System#nanotime Changed init to not suppress the first lock warning if the first acquisition is within the log delay period Changed logging check to avoid synchronizing on the instance Changed the unit test to use Timer instead of Thread#sleep This retains most of the pattern as requested .
          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.
          0 mvndep 1m 23s Maven dependency ordering for branch
          +1 mvninstall 7m 35s trunk passed
          +1 compile 7m 41s trunk passed
          +1 checkstyle 1m 28s trunk passed
          +1 mvnsite 2m 5s trunk passed
          +1 mvneclipse 0m 29s trunk passed
          +1 findbugs 3m 14s trunk passed
          +1 javadoc 1m 51s trunk passed
          0 mvndep 0m 15s Maven dependency ordering for patch
          +1 mvninstall 1m 39s the patch passed
          +1 compile 7m 30s the patch passed
          +1 javac 7m 30s the patch passed
          +1 checkstyle 1m 25s the patch passed
          +1 mvnsite 1m 50s the patch passed
          +1 mvneclipse 0m 25s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 20s the patch passed
          +1 javadoc 1m 40s the patch passed
          +1 unit 8m 27s hadoop-common in the patch passed.
          -1 unit 58m 51s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 23s The patch does not generate ASF License warnings.
          112m 38s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.namenode.ha.TestHASafeMode
            hadoop.hdfs.TestRenameWhileOpen
            hadoop.hdfs.TestDecommissionWithStriped



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10742
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827323/HDFS-10742.015.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux abc5f0859ea4 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 / c0e492e
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16655/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16655/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16655/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. 0 mvndep 1m 23s Maven dependency ordering for branch +1 mvninstall 7m 35s trunk passed +1 compile 7m 41s trunk passed +1 checkstyle 1m 28s trunk passed +1 mvnsite 2m 5s trunk passed +1 mvneclipse 0m 29s trunk passed +1 findbugs 3m 14s trunk passed +1 javadoc 1m 51s trunk passed 0 mvndep 0m 15s Maven dependency ordering for patch +1 mvninstall 1m 39s the patch passed +1 compile 7m 30s the patch passed +1 javac 7m 30s the patch passed +1 checkstyle 1m 25s the patch passed +1 mvnsite 1m 50s the patch passed +1 mvneclipse 0m 25s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 20s the patch passed +1 javadoc 1m 40s the patch passed +1 unit 8m 27s hadoop-common in the patch passed. -1 unit 58m 51s hadoop-hdfs in the patch failed. +1 asflicense 0m 23s The patch does not generate ASF License warnings. 112m 38s Reason Tests Failed junit tests hadoop.hdfs.server.namenode.ha.TestHASafeMode   hadoop.hdfs.TestRenameWhileOpen   hadoop.hdfs.TestDecommissionWithStriped Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10742 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827323/HDFS-10742.015.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux abc5f0859ea4 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 / c0e492e Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/16655/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16655/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16655/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          vagarychen Chen Liang added a comment -

          v015 patch looks to be a better one to me compared to my latest patch v014. Thanks Chris Douglas for the patch!

          Show
          vagarychen Chen Liang added a comment - v015 patch looks to be a better one to me compared to my latest patch v014. Thanks Chris Douglas for the patch!
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Thank you Chris Douglas. Your patch lgtm.

          I remain skeptical of AutoCloseableLock, since one could just use the Lock in interface and existing tooling to implement the same. But if its virtues are obvious to everyone but me I won't stand in the way of including it.

          As you said its syntactic sugar. If this approach feels that inelegant I am okay with eliminating AutoCloseableLock.

          Second, minutiae like "GC-friendliness", whether a wrapper class on Lock adds dispatch overhead, class loading overhead, etc. when this JIRA logs critical sections longer than 300ms are irrelevant, literally by orders of magnitude.

          Moot point now but iiuc the object would have been allocated on every lock acquire whether we logged or not. Perhaps the overhead would have been negligible but I'm not sure myself.

          Show
          arpitagarwal Arpit Agarwal added a comment - Thank you Chris Douglas . Your patch lgtm. I remain skeptical of AutoCloseableLock, since one could just use the Lock in interface and existing tooling to implement the same. But if its virtues are obvious to everyone but me I won't stand in the way of including it. As you said its syntactic sugar. If this approach feels that inelegant I am okay with eliminating AutoCloseableLock . Second, minutiae like "GC-friendliness", whether a wrapper class on Lock adds dispatch overhead, class loading overhead, etc. when this JIRA logs critical sections longer than 300ms are irrelevant, literally by orders of magnitude. Moot point now but iiuc the object would have been allocated on every lock acquire whether we logged or not. Perhaps the overhead would have been negligible but I'm not sure myself.
          Hide
          chris.douglas Chris Douglas added a comment -

          v016:

          • Avoid a race condition recording the last-logged timestamp.
          • Set suppression interval to be configurable with a default of 10s

          v016 doesn't set the 300ms interval to be configurable, neither does it separate the log from FsDatasetImpl.

          As you said its syntactic sugar. If this approach feels that inelegant I am okay with eliminating AutoCloseableLock.

          shrug I've stated my case against it, but if you're not convinced and prefer to retain it: no big deal.

          Show
          chris.douglas Chris Douglas added a comment - v016: Avoid a race condition recording the last-logged timestamp. Set suppression interval to be configurable with a default of 10s v016 doesn't set the 300ms interval to be configurable, neither does it separate the log from FsDatasetImpl. As you said its syntactic sugar. If this approach feels that inelegant I am okay with eliminating AutoCloseableLock. shrug I've stated my case against it, but if you're not convinced and prefer to retain it: no big deal.
          Hide
          xkrogen Erik Krogen added a comment - - edited

          For v16 patch IIUC you actually still have the default set at 2s (FsDatasetImpl line 287). Also all of the other time keys in DFSConfigKeys use a numeric value (of ms or s) rather than a string representation of time, should we keep consistency? Lastly should the new config also be added to hdfs-default.xml?

          Show
          xkrogen Erik Krogen added a comment - - edited For v16 patch IIUC you actually still have the default set at 2s (FsDatasetImpl line 287). Also all of the other time keys in DFSConfigKeys use a numeric value (of ms or s) rather than a string representation of time, should we keep consistency? Lastly should the new config also be added to hdfs-default.xml ?
          Hide
          chris.douglas Chris Douglas added a comment -

          Good catch, sorry forgot to reference the default.

          I was trying to follow guidance in HDFS-9847 on the default type, but either way. Changed to long.

          Show
          chris.douglas Chris Douglas added a comment - Good catch, sorry forgot to reference the default. I was trying to follow guidance in HDFS-9847 on the default type, but either way. Changed to long .
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          +1 for the v017 patch.

          Show
          arpitagarwal Arpit Agarwal added a comment - +1 for the v017 patch.
          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.
          0 mvndep 0m 19s Maven dependency ordering for branch
          +1 mvninstall 9m 18s trunk passed
          +1 compile 9m 56s trunk passed
          +1 checkstyle 1m 58s trunk passed
          +1 mvnsite 2m 37s trunk passed
          +1 mvneclipse 0m 34s trunk passed
          +1 findbugs 3m 55s trunk passed
          +1 javadoc 2m 13s trunk passed
          0 mvndep 0m 18s Maven dependency ordering for patch
          +1 mvninstall 2m 7s the patch passed
          +1 compile 10m 8s the patch passed
          +1 javac 10m 8s the patch passed
          +1 checkstyle 1m 31s the patch passed
          +1 mvnsite 1m 57s the patch passed
          +1 mvneclipse 0m 30s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 26s the patch passed
          +1 javadoc 1m 48s the patch passed
          -1 unit 19m 54s hadoop-common in the patch failed.
          -1 unit 84m 35s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 30s The patch does not generate ASF License warnings.
          159m 1s



          Reason Tests
          Failed junit tests hadoop.tools.TestHdfsConfigFields
            hadoop.hdfs.server.namenode.snapshot.TestSnapshotFileLength
            hadoop.hdfs.TestDFSClientRetries
          Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10742
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827647/HDFS-10742.016.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux a1f148db0c99 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 / b6d839a
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16679/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/16679/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16679/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16679/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. 0 mvndep 0m 19s Maven dependency ordering for branch +1 mvninstall 9m 18s trunk passed +1 compile 9m 56s trunk passed +1 checkstyle 1m 58s trunk passed +1 mvnsite 2m 37s trunk passed +1 mvneclipse 0m 34s trunk passed +1 findbugs 3m 55s trunk passed +1 javadoc 2m 13s trunk passed 0 mvndep 0m 18s Maven dependency ordering for patch +1 mvninstall 2m 7s the patch passed +1 compile 10m 8s the patch passed +1 javac 10m 8s the patch passed +1 checkstyle 1m 31s the patch passed +1 mvnsite 1m 57s the patch passed +1 mvneclipse 0m 30s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 26s the patch passed +1 javadoc 1m 48s the patch passed -1 unit 19m 54s hadoop-common in the patch failed. -1 unit 84m 35s hadoop-hdfs in the patch failed. +1 asflicense 0m 30s The patch does not generate ASF License warnings. 159m 1s Reason Tests Failed junit tests hadoop.tools.TestHdfsConfigFields   hadoop.hdfs.server.namenode.snapshot.TestSnapshotFileLength   hadoop.hdfs.TestDFSClientRetries Timed out junit tests org.apache.hadoop.http.TestHttpServerLifecycle Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10742 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827647/HDFS-10742.016.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a1f148db0c99 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 / b6d839a Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/16679/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/16679/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16679/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16679/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          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 1 new or modified test files.
          0 mvndep 1m 34s Maven dependency ordering for branch
          +1 mvninstall 8m 52s trunk passed
          +1 compile 8m 3s trunk passed
          +1 checkstyle 1m 31s trunk passed
          +1 mvnsite 1m 50s trunk passed
          +1 mvneclipse 0m 27s trunk passed
          +1 findbugs 3m 2s trunk passed
          +1 javadoc 1m 38s trunk passed
          0 mvndep 0m 14s Maven dependency ordering for patch
          +1 mvninstall 1m 24s the patch passed
          +1 compile 6m 41s the patch passed
          +1 javac 6m 42s the patch passed
          +1 checkstyle 1m 27s the patch passed
          +1 mvnsite 1m 42s the patch passed
          +1 mvneclipse 0m 25s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 3m 13s the patch passed
          +1 javadoc 1m 38s the patch passed
          +1 unit 7m 36s hadoop-common in the patch passed.
          +1 unit 62m 29s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 25s The patch does not generate ASF License warnings.
          115m 21s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HDFS-10742
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827659/HDFS-10742.017.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 139e5f52d510 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 / b6d839a
          Default Java 1.8.0_101
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16682/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16682/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 1 new or modified test files. 0 mvndep 1m 34s Maven dependency ordering for branch +1 mvninstall 8m 52s trunk passed +1 compile 8m 3s trunk passed +1 checkstyle 1m 31s trunk passed +1 mvnsite 1m 50s trunk passed +1 mvneclipse 0m 27s trunk passed +1 findbugs 3m 2s trunk passed +1 javadoc 1m 38s trunk passed 0 mvndep 0m 14s Maven dependency ordering for patch +1 mvninstall 1m 24s the patch passed +1 compile 6m 41s the patch passed +1 javac 6m 42s the patch passed +1 checkstyle 1m 27s the patch passed +1 mvnsite 1m 42s the patch passed +1 mvneclipse 0m 25s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 3m 13s the patch passed +1 javadoc 1m 38s the patch passed +1 unit 7m 36s hadoop-common in the patch passed. +1 unit 62m 29s hadoop-hdfs in the patch passed. +1 asflicense 0m 25s The patch does not generate ASF License warnings. 115m 21s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HDFS-10742 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12827659/HDFS-10742.017.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 139e5f52d510 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 / b6d839a Default Java 1.8.0_101 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/16682/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HDFS-Build/16682/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 -

          I committed this. Thanks Chen

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

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10414 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10414/)
          HDFS-10742. Measure lock time in FsDatasetImpl. Contributed by Chen (cdouglas: rev 011f3b24d4bfda505a90ab5b5576916a41f869c5)

          • (add) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestInstrumentedLock.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java
          • (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/AutoCloseableLock.java
          • (add) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/InstrumentedLock.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java
          • (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10414 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10414/ ) HDFS-10742 . Measure lock time in FsDatasetImpl. Contributed by Chen (cdouglas: rev 011f3b24d4bfda505a90ab5b5576916a41f869c5) (add) hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestInstrumentedLock.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/fsdataset/impl/FsDatasetImpl.java (edit) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/AutoCloseableLock.java (add) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/InstrumentedLock.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/DFSConfigKeys.java (edit) hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Committed to branch-2 and branch-2.8 after local unit test run.

          Show
          arpitagarwal Arpit Agarwal added a comment - Committed to branch-2 and branch-2.8 after local unit test run.

            People

            • Assignee:
              vagarychen Chen Liang
              Reporter:
              vagarychen Chen Liang
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development