Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-21342

FileSystem in use may get closed by other bulk load call in secure bulkLoad

Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 3.0.0-alpha-1, 2.1.0, 1.5.0, 1.3.3, 1.4.4, 2.0.1, 1.2.7
    • 3.0.0-alpha-1, 2.2.0, 2.1.1, 2.0.3
    • None
    • None

    Description

      As mentioned in HBASE-15291, there is a race condition.   If Two secure bulkload calls  from the same UGI into two different regions and one region finishes earlier, it will close the bulk load fs, and the other region will fail.

       

      Another case would be more serious. The FileSystem.close() function needs two synchronized variables : CACHE and deleteOnExit. If one region calls FileSystem.closeAllForUGI ( in SecureBulkLoadManager.cleanupBulkLoad) while another region is trying to close srcFS ( in  SecureBulkLoadListener.closeSrcFs)   , can cause deadlock here.

       

      I have wrote a UT for this and fixed it using reference counter.

       

      Attachments

        1. 21342.v1.txt
          12 kB
          Ted Yu
        2. HBASE-21342.002.patch
          12 kB
          mazhenlin
        3. HBASE-21342.003.patch
          13 kB
          mazhenlin
        4. HBASE-21342.004.patch
          14 kB
          mazhenlin
        5. HBASE-21342.005.patch
          14 kB
          mazhenlin
        6. HBASE-21342.006.patch
          15 kB
          mazhenlin
        7. HBASE-21342.007.patch
          16 kB
          mazhenlin
        8. race.patch
          12 kB
          mazhenlin

        Issue Links

          Activity

            mdrob Mike Drob added a comment -

            mazhenlin - I have added you to our contributor's group in JIRA, you should be able to assign yourself the issue and upload patches/submit to Jenkins for QA now!

            mdrob Mike Drob added a comment - mazhenlin - I have added you to our contributor's group in JIRA, you should be able to assign yourself the issue and upload patches/submit to Jenkins for QA now!
            yuzhihong@gmail.com Ted Yu added a comment -
            +import org.apache.commons.collections.map.HashedMap;
            

            Did you intend to import HashMap instead ?

            + * Created by mazhenlin on 2018/10/18.
            

            Please drop author.

            +public class TestSecureBulkloadManager implements InternalObserverForTest {
            

            Missing test category.
            The test also misses @ClassRule e.g.

              @ClassRule
              public static final HBaseClassTestRule CLASS_RULE =
                  HBaseClassTestRule.forClass(TestCopyTable.class);
            
            yuzhihong@gmail.com Ted Yu added a comment - + import org.apache.commons.collections.map.HashedMap; Did you intend to import HashMap instead ? + * Created by mazhenlin on 2018/10/18. Please drop author. + public class TestSecureBulkloadManager implements InternalObserverForTest { Missing test category. The test also misses @ClassRule e.g. @ClassRule public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestCopyTable.class);
            mazhenlin mazhenlin added a comment -

            mdrob Thank you. I will submit my patch tomorrow. Right now I have not figured out how to submit

            mazhenlin mazhenlin added a comment - mdrob Thank you. I will submit my patch tomorrow. Right now I have not figured out how to submit
            mdrob Mike Drob added a comment -

            + private static byte[] value2 = Bytes.toBytes("t2");
            + private static byte[] value3 = Bytes.toBytes("t2");

            These are the same.

            We should probably be using ConcurrentHashMap and the various concurrency functions there instead of making our own big synchronized blocks. Looking at putIfAbsent, computeIfAbsent, compute, and merge in particular.

            When I try to run the test on master branch I get java.lang.ArrayIndexOutOfBoundsException: 0 so something is going wrong there... which branch did you work on this for initially?

            mdrob Mike Drob added a comment - + private static byte[] value2 = Bytes.toBytes("t2"); + private static byte[] value3 = Bytes.toBytes("t2"); These are the same. We should probably be using ConcurrentHashMap and the various concurrency functions there instead of making our own big synchronized blocks. Looking at putIfAbsent, computeIfAbsent, compute, and merge in particular. When I try to run the test on master branch I get java.lang.ArrayIndexOutOfBoundsException: 0 so something is going wrong there... which branch did you work on this for initially?
            yuzhihong@gmail.com Ted Yu added a comment -

            Added ClassRule to TestSecureBulkloadManager.

            Let's see what QA bot says.

            Zhenlin:
            Please add Apache license header to TestSecureBulkloadManager in your next patch.

            yuzhihong@gmail.com Ted Yu added a comment - Added ClassRule to TestSecureBulkloadManager. Let's see what QA bot says. Zhenlin: Please add Apache license header to TestSecureBulkloadManager in your next patch.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 11s Docker mode activated.
            0 patch 0m 2s The patch file was not named according to hbase's naming conventions. Please see https://yetus.apache.org/documentation/0.8.0/precommit-patchnames for instructions.
                  Prechecks
            +1 hbaseanti 0m 0s Patch does not have any anti-patterns.
            +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.
                  master Compile Tests
            +1 mvninstall 5m 41s master passed
            +1 compile 1m 57s master passed
            +1 checkstyle 1m 17s master passed
            +1 shadedjars 4m 31s branch has no errors when building our shaded downstream artifacts.
            +1 findbugs 2m 0s master passed
            +1 javadoc 0m 30s master passed
                  Patch Compile Tests
            +1 mvninstall 5m 15s the patch passed
            +1 compile 1m 47s the patch passed
            -1 javac 1m 47s hbase-server generated 3 new + 185 unchanged - 3 fixed = 188 total (was 188)
            -1 checkstyle 1m 13s hbase-server: The patch generated 29 new + 3 unchanged - 0 fixed = 32 total (was 3)
            +1 whitespace 0m 0s The patch has no whitespace issues.
            -1 shadedjars 3m 21s patch has 10 errors when building our shaded downstream artifacts.
            +1 hadoopcheck 10m 20s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
            +1 findbugs 2m 1s the patch passed
            +1 javadoc 0m 30s the patch passed
                  Other Tests
            -1 unit 132m 34s hbase-server in the patch failed.
            -1 asflicense 0m 25s The patch generated 1 ASF License warnings.
            174m 8s



            Reason Tests
            Failed junit tests hadoop.hbase.replication.TestReplicationDroppedTables
              hadoop.hbase.TestCheckTestClasses



            Subsystem Report/Notes
            Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b
            JIRA Issue HBASE-21342
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12944661/21342.v1.txt
            Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
            uname Linux a9ef520180ed 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 GNU/Linux
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh
            git revision master / 05d22ed960
            maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
            Default Java 1.8.0_181
            findbugs v3.1.0-RC3
            javac https://builds.apache.org/job/PreCommit-HBASE-Build/14751/artifact/patchprocess/diff-compile-javac-hbase-server.txt
            checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/14751/artifact/patchprocess/diff-checkstyle-hbase-server.txt
            shadedjars https://builds.apache.org/job/PreCommit-HBASE-Build/14751/artifact/patchprocess/patch-shadedjars.txt
            unit https://builds.apache.org/job/PreCommit-HBASE-Build/14751/artifact/patchprocess/patch-unit-hbase-server.txt
            Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14751/testReport/
            asflicense https://builds.apache.org/job/PreCommit-HBASE-Build/14751/artifact/patchprocess/patch-asflicense-problems.txt
            Max. process+thread count 4607 (vs. ulimit of 10000)
            modules C: hbase-server U: hbase-server
            Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14751/console
            Powered by Apache Yetus 0.8.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. 0 patch 0m 2s The patch file was not named according to hbase's naming conventions. Please see https://yetus.apache.org/documentation/0.8.0/precommit-patchnames for instructions.       Prechecks +1 hbaseanti 0m 0s Patch does not have any anti-patterns. +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.       master Compile Tests +1 mvninstall 5m 41s master passed +1 compile 1m 57s master passed +1 checkstyle 1m 17s master passed +1 shadedjars 4m 31s branch has no errors when building our shaded downstream artifacts. +1 findbugs 2m 0s master passed +1 javadoc 0m 30s master passed       Patch Compile Tests +1 mvninstall 5m 15s the patch passed +1 compile 1m 47s the patch passed -1 javac 1m 47s hbase-server generated 3 new + 185 unchanged - 3 fixed = 188 total (was 188) -1 checkstyle 1m 13s hbase-server: The patch generated 29 new + 3 unchanged - 0 fixed = 32 total (was 3) +1 whitespace 0m 0s The patch has no whitespace issues. -1 shadedjars 3m 21s patch has 10 errors when building our shaded downstream artifacts. +1 hadoopcheck 10m 20s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0. +1 findbugs 2m 1s the patch passed +1 javadoc 0m 30s the patch passed       Other Tests -1 unit 132m 34s hbase-server in the patch failed. -1 asflicense 0m 25s The patch generated 1 ASF License warnings. 174m 8s Reason Tests Failed junit tests hadoop.hbase.replication.TestReplicationDroppedTables   hadoop.hbase.TestCheckTestClasses Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b JIRA Issue HBASE-21342 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12944661/21342.v1.txt Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile uname Linux a9ef520180ed 3.13.0-139-generic #188-Ubuntu SMP Tue Jan 9 14:43:09 UTC 2018 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh git revision master / 05d22ed960 maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z) Default Java 1.8.0_181 findbugs v3.1.0-RC3 javac https://builds.apache.org/job/PreCommit-HBASE-Build/14751/artifact/patchprocess/diff-compile-javac-hbase-server.txt checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/14751/artifact/patchprocess/diff-checkstyle-hbase-server.txt shadedjars https://builds.apache.org/job/PreCommit-HBASE-Build/14751/artifact/patchprocess/patch-shadedjars.txt unit https://builds.apache.org/job/PreCommit-HBASE-Build/14751/artifact/patchprocess/patch-unit-hbase-server.txt Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14751/testReport/ asflicense https://builds.apache.org/job/PreCommit-HBASE-Build/14751/artifact/patchprocess/patch-asflicense-problems.txt Max. process+thread count 4607 (vs. ulimit of 10000) modules C: hbase-server U: hbase-server Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14751/console Powered by Apache Yetus 0.8.0 http://yetus.apache.org This message was automatically generated.
            yuzhihong@gmail.com Ted Yu added a comment -

            Can you change the name of the test table to reflect the nature of the test ?

            Thanks

            yuzhihong@gmail.com Ted Yu added a comment - Can you change the name of the test table to reflect the nature of the test ? Thanks
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 11s Docker mode activated.
                  Prechecks
            +1 hbaseanti 0m 0s Patch does not have any anti-patterns.
            +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.
                  master Compile Tests
            +1 mvninstall 5m 17s master passed
            +1 compile 1m 48s master passed
            +1 checkstyle 1m 14s master passed
            +1 shadedjars 4m 30s branch has no errors when building our shaded downstream artifacts.
            +1 findbugs 2m 27s master passed
            +1 javadoc 0m 36s master passed
                  Patch Compile Tests
            +1 mvninstall 7m 6s the patch passed
            +1 compile 3m 3s the patch passed
            +1 javac 3m 3s the patch passed
            -1 checkstyle 1m 58s hbase-server: The patch generated 21 new + 3 unchanged - 0 fixed = 24 total (was 3)
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 shadedjars 6m 59s patch has no errors when building our shaded downstream artifacts.
            +1 hadoopcheck 18m 30s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
            +1 findbugs 4m 48s the patch passed
            +1 javadoc 1m 15s the patch passed
                  Other Tests
            +1 unit 140m 27s hbase-server in the patch passed.
            +1 asflicense 0m 24s The patch does not generate ASF License warnings.
            201m 26s



            Subsystem Report/Notes
            Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b
            JIRA Issue HBASE-21342
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12944670/HBASE-21342.002.patch
            Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
            uname Linux 0e006bc17937 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 10:45:36 UTC 2018 x86_64 GNU/Linux
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh
            git revision master / 05d22ed960
            maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
            Default Java 1.8.0_181
            findbugs v3.1.0-RC3
            checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/14752/artifact/patchprocess/diff-checkstyle-hbase-server.txt
            Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14752/testReport/
            Max. process+thread count 5040 (vs. ulimit of 10000)
            modules C: hbase-server U: hbase-server
            Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14752/console
            Powered by Apache Yetus 0.8.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated.       Prechecks +1 hbaseanti 0m 0s Patch does not have any anti-patterns. +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.       master Compile Tests +1 mvninstall 5m 17s master passed +1 compile 1m 48s master passed +1 checkstyle 1m 14s master passed +1 shadedjars 4m 30s branch has no errors when building our shaded downstream artifacts. +1 findbugs 2m 27s master passed +1 javadoc 0m 36s master passed       Patch Compile Tests +1 mvninstall 7m 6s the patch passed +1 compile 3m 3s the patch passed +1 javac 3m 3s the patch passed -1 checkstyle 1m 58s hbase-server: The patch generated 21 new + 3 unchanged - 0 fixed = 24 total (was 3) +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedjars 6m 59s patch has no errors when building our shaded downstream artifacts. +1 hadoopcheck 18m 30s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0. +1 findbugs 4m 48s the patch passed +1 javadoc 1m 15s the patch passed       Other Tests +1 unit 140m 27s hbase-server in the patch passed. +1 asflicense 0m 24s The patch does not generate ASF License warnings. 201m 26s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b JIRA Issue HBASE-21342 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12944670/HBASE-21342.002.patch Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile uname Linux 0e006bc17937 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 10:45:36 UTC 2018 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh git revision master / 05d22ed960 maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z) Default Java 1.8.0_181 findbugs v3.1.0-RC3 checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/14752/artifact/patchprocess/diff-checkstyle-hbase-server.txt Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14752/testReport/ Max. process+thread count 5040 (vs. ulimit of 10000) modules C: hbase-server U: hbase-server Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14752/console Powered by Apache Yetus 0.8.0 http://yetus.apache.org This message was automatically generated.
            yuzhihong@gmail.com Ted Yu added a comment -
            +  interface InternalObserverForTest {
            

            Since the interface has VisibleForTesting annotation, the interface name doesn't have to include ForTest.
            How about naming the interface InternalObserverForFSOperation or something similar ?

            +      decrementUgiReference(ugi);
            

            Should the above be enclosed in finally block ?

            +  public void testForRaceCondition() throws Exception {
            

            There may be more test added to this class in the future. If including the race condition in test name makes the method name too long, can you add a comment describing the race ?

            +  class MyExceptionToAvoidRetry extends DoNotRetryIOException {};
            

            Put the right curly on separate line.

            +      e.printStackTrace();
            

            Please replace with a rethrow of the exception.

            +        Thread.sleep(3000); /// sleep 3s so the fs will be closed by the time we wakeup
            

            Is it possible to use some other condition so that the wait is not hardcoded ?
            Sometime down the road, what if 3 second is not long enough ?

            +      } catch (InterruptedException e) {}
            

            Please log and rethrow InterruptedIOException.

            thanks for the great work.

            yuzhihong@gmail.com Ted Yu added a comment - + interface InternalObserverForTest { Since the interface has VisibleForTesting annotation, the interface name doesn't have to include ForTest. How about naming the interface InternalObserverForFSOperation or something similar ? + decrementUgiReference(ugi); Should the above be enclosed in finally block ? + public void testForRaceCondition() throws Exception { There may be more test added to this class in the future. If including the race condition in test name makes the method name too long, can you add a comment describing the race ? + class MyExceptionToAvoidRetry extends DoNotRetryIOException {}; Put the right curly on separate line. + e.printStackTrace(); Please replace with a rethrow of the exception. + Thread .sleep(3000); /// sleep 3s so the fs will be closed by the time we wakeup Is it possible to use some other condition so that the wait is not hardcoded ? Sometime down the road, what if 3 second is not long enough ? + } catch (InterruptedException e) {} Please log and rethrow InterruptedIOException. thanks for the great work.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 28s Docker mode activated.
                  Prechecks
            +1 hbaseanti 0m 0s Patch does not have any anti-patterns.
            +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.
                  master Compile Tests
            +1 mvninstall 8m 54s master passed
            +1 compile 1m 57s master passed
            +1 checkstyle 1m 24s master passed
            +1 shadedjars 4m 54s branch has no errors when building our shaded downstream artifacts.
            +1 findbugs 2m 13s master passed
            +1 javadoc 0m 42s master passed
                  Patch Compile Tests
            +1 mvninstall 5m 55s the patch passed
            +1 compile 2m 2s the patch passed
            +1 javac 2m 2s the patch passed
            -1 checkstyle 1m 20s hbase-server: The patch generated 7 new + 3 unchanged - 0 fixed = 10 total (was 3)
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 shadedjars 4m 40s patch has no errors when building our shaded downstream artifacts.
            +1 hadoopcheck 10m 57s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
            +1 findbugs 2m 10s the patch passed
            +1 javadoc 0m 29s the patch passed
                  Other Tests
            -1 unit 229m 36s hbase-server in the patch failed.
            +1 asflicense 0m 40s The patch does not generate ASF License warnings.
            279m 12s



            Reason Tests
            Failed junit tests hadoop.hbase.replication.multiwal.TestReplicationSyncUpToolWithMultipleWAL



            Subsystem Report/Notes
            Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b
            JIRA Issue HBASE-21342
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12944714/HBASE-21342.003.patch
            Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
            uname Linux 916f234338f2 3.13.0-153-generic #203-Ubuntu SMP Thu Jun 14 08:52:28 UTC 2018 x86_64 GNU/Linux
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh
            git revision master / 05d22ed960
            maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
            Default Java 1.8.0_181
            findbugs v3.1.0-RC3
            checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/14755/artifact/patchprocess/diff-checkstyle-hbase-server.txt
            unit https://builds.apache.org/job/PreCommit-HBASE-Build/14755/artifact/patchprocess/patch-unit-hbase-server.txt
            Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14755/testReport/
            Max. process+thread count 4410 (vs. ulimit of 10000)
            modules C: hbase-server U: hbase-server
            Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14755/console
            Powered by Apache Yetus 0.8.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 28s Docker mode activated.       Prechecks +1 hbaseanti 0m 0s Patch does not have any anti-patterns. +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.       master Compile Tests +1 mvninstall 8m 54s master passed +1 compile 1m 57s master passed +1 checkstyle 1m 24s master passed +1 shadedjars 4m 54s branch has no errors when building our shaded downstream artifacts. +1 findbugs 2m 13s master passed +1 javadoc 0m 42s master passed       Patch Compile Tests +1 mvninstall 5m 55s the patch passed +1 compile 2m 2s the patch passed +1 javac 2m 2s the patch passed -1 checkstyle 1m 20s hbase-server: The patch generated 7 new + 3 unchanged - 0 fixed = 10 total (was 3) +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedjars 4m 40s patch has no errors when building our shaded downstream artifacts. +1 hadoopcheck 10m 57s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0. +1 findbugs 2m 10s the patch passed +1 javadoc 0m 29s the patch passed       Other Tests -1 unit 229m 36s hbase-server in the patch failed. +1 asflicense 0m 40s The patch does not generate ASF License warnings. 279m 12s Reason Tests Failed junit tests hadoop.hbase.replication.multiwal.TestReplicationSyncUpToolWithMultipleWAL Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b JIRA Issue HBASE-21342 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12944714/HBASE-21342.003.patch Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile uname Linux 916f234338f2 3.13.0-153-generic #203-Ubuntu SMP Thu Jun 14 08:52:28 UTC 2018 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh git revision master / 05d22ed960 maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z) Default Java 1.8.0_181 findbugs v3.1.0-RC3 checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/14755/artifact/patchprocess/diff-checkstyle-hbase-server.txt unit https://builds.apache.org/job/PreCommit-HBASE-Build/14755/artifact/patchprocess/patch-unit-hbase-server.txt Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14755/testReport/ Max. process+thread count 4410 (vs. ulimit of 10000) modules C: hbase-server U: hbase-server Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14755/console Powered by Apache Yetus 0.8.0 http://yetus.apache.org This message was automatically generated.
            mazhenlin mazhenlin added a comment -

            Should the above be enclosed in finally block ?

            It is in finally block.

             

            Other comments were fixed. Thank you very much for the review, I have learned a lot.

            mazhenlin mazhenlin added a comment - Should the above be enclosed in finally block ? It is in finally block.   Other comments were fixed. Thank you very much for the review, I have learned a lot.
            yuzhihong@gmail.com Ted Yu added a comment -

            It is in finally block.

            Perhaps I was not very clear in the previous comment.

            postBulkLoadHFile() throws IOE. When this happens, I don't think Java would run the rest of the code (decrementUgiReference in this case) in the finally block.
            You can either move decrementUgiReference call to the beginning of finally block (since it doesn't throw IOE) or, use nested finally in the current finally block.

            yuzhihong@gmail.com Ted Yu added a comment - It is in finally block. Perhaps I was not very clear in the previous comment. postBulkLoadHFile() throws IOE. When this happens, I don't think Java would run the rest of the code (decrementUgiReference in this case) in the finally block. You can either move decrementUgiReference call to the beginning of finally block (since it doesn't throw IOE) or, use nested finally in the current finally block.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 11s Docker mode activated.
                  Prechecks
            +1 hbaseanti 0m 0s Patch does not have any anti-patterns.
            +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.
                  master Compile Tests
            +1 mvninstall 5m 12s master passed
            +1 compile 1m 45s master passed
            +1 checkstyle 1m 12s master passed
            +1 shadedjars 4m 14s branch has no errors when building our shaded downstream artifacts.
            +1 findbugs 1m 54s master passed
            +1 javadoc 0m 29s master passed
                  Patch Compile Tests
            +1 mvninstall 5m 3s the patch passed
            +1 compile 1m 45s the patch passed
            +1 javac 1m 45s the patch passed
            -1 checkstyle 1m 10s hbase-server: The patch generated 3 new + 3 unchanged - 0 fixed = 6 total (was 3)
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 shadedjars 4m 15s patch has no errors when building our shaded downstream artifacts.
            +1 hadoopcheck 10m 50s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
            +1 findbugs 2m 19s the patch passed
            +1 javadoc 0m 31s the patch passed
                  Other Tests
            -1 unit 143m 36s hbase-server in the patch failed.
            +1 asflicense 0m 27s The patch does not generate ASF License warnings.
            185m 26s



            Reason Tests
            Failed junit tests hadoop.hbase.client.TestBlockEvictionFromClient



            Subsystem Report/Notes
            Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b
            JIRA Issue HBASE-21342
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12944908/HBASE-21342.004.patch
            Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
            uname Linux 5a0d9b55f3dc 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 10:45:36 UTC 2018 x86_64 GNU/Linux
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh
            git revision master / dd474ef199
            maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
            Default Java 1.8.0_181
            findbugs v3.1.0-RC3
            checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/14785/artifact/patchprocess/diff-checkstyle-hbase-server.txt
            unit https://builds.apache.org/job/PreCommit-HBASE-Build/14785/artifact/patchprocess/patch-unit-hbase-server.txt
            Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14785/testReport/
            Max. process+thread count 4546 (vs. ulimit of 10000)
            modules C: hbase-server U: hbase-server
            Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14785/console
            Powered by Apache Yetus 0.8.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated.       Prechecks +1 hbaseanti 0m 0s Patch does not have any anti-patterns. +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.       master Compile Tests +1 mvninstall 5m 12s master passed +1 compile 1m 45s master passed +1 checkstyle 1m 12s master passed +1 shadedjars 4m 14s branch has no errors when building our shaded downstream artifacts. +1 findbugs 1m 54s master passed +1 javadoc 0m 29s master passed       Patch Compile Tests +1 mvninstall 5m 3s the patch passed +1 compile 1m 45s the patch passed +1 javac 1m 45s the patch passed -1 checkstyle 1m 10s hbase-server: The patch generated 3 new + 3 unchanged - 0 fixed = 6 total (was 3) +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedjars 4m 15s patch has no errors when building our shaded downstream artifacts. +1 hadoopcheck 10m 50s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0. +1 findbugs 2m 19s the patch passed +1 javadoc 0m 31s the patch passed       Other Tests -1 unit 143m 36s hbase-server in the patch failed. +1 asflicense 0m 27s The patch does not generate ASF License warnings. 185m 26s Reason Tests Failed junit tests hadoop.hbase.client.TestBlockEvictionFromClient Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b JIRA Issue HBASE-21342 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12944908/HBASE-21342.004.patch Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile uname Linux 5a0d9b55f3dc 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 10:45:36 UTC 2018 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh git revision master / dd474ef199 maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z) Default Java 1.8.0_181 findbugs v3.1.0-RC3 checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/14785/artifact/patchprocess/diff-checkstyle-hbase-server.txt unit https://builds.apache.org/job/PreCommit-HBASE-Build/14785/artifact/patchprocess/patch-unit-hbase-server.txt Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14785/testReport/ Max. process+thread count 4546 (vs. ulimit of 10000) modules C: hbase-server U: hbase-server Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14785/console Powered by Apache Yetus 0.8.0 http://yetus.apache.org This message was automatically generated.
            hadoopqa Hadoop QA added a comment -
            -1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 12s Docker mode activated.
                  Prechecks
            +1 hbaseanti 0m 0s Patch does not have any anti-patterns.
            +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.
                  master Compile Tests
            +1 mvninstall 5m 30s master passed
            +1 compile 1m 50s master passed
            +1 checkstyle 1m 17s master passed
            +1 shadedjars 4m 32s branch has no errors when building our shaded downstream artifacts.
            +1 findbugs 2m 4s master passed
            +1 javadoc 0m 33s master passed
                  Patch Compile Tests
            +1 mvninstall 5m 17s the patch passed
            +1 compile 1m 51s the patch passed
            +1 javac 1m 51s the patch passed
            -1 checkstyle 1m 14s hbase-server: The patch generated 3 new + 3 unchanged - 0 fixed = 6 total (was 3)
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 shadedjars 4m 22s patch has no errors when building our shaded downstream artifacts.
            +1 hadoopcheck 11m 51s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
            +1 findbugs 2m 49s the patch passed
            +1 javadoc 0m 41s the patch passed
                  Other Tests
            +1 unit 145m 16s hbase-server in the patch passed.
            +1 asflicense 0m 30s The patch does not generate ASF License warnings.
            190m 27s



            Subsystem Report/Notes
            Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b
            JIRA Issue HBASE-21342
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12944909/HBASE-21342.005.patch
            Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
            uname Linux 2f5b93405f3e 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 10:45:36 UTC 2018 x86_64 GNU/Linux
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build@2/component/dev-support/hbase-personality.sh
            git revision master / dd474ef199
            maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
            Default Java 1.8.0_181
            findbugs v3.1.0-RC3
            checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/14786/artifact/patchprocess/diff-checkstyle-hbase-server.txt
            Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14786/testReport/
            Max. process+thread count 4679 (vs. ulimit of 10000)
            modules C: hbase-server U: hbase-server
            Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14786/console
            Powered by Apache Yetus 0.8.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 12s Docker mode activated.       Prechecks +1 hbaseanti 0m 0s Patch does not have any anti-patterns. +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.       master Compile Tests +1 mvninstall 5m 30s master passed +1 compile 1m 50s master passed +1 checkstyle 1m 17s master passed +1 shadedjars 4m 32s branch has no errors when building our shaded downstream artifacts. +1 findbugs 2m 4s master passed +1 javadoc 0m 33s master passed       Patch Compile Tests +1 mvninstall 5m 17s the patch passed +1 compile 1m 51s the patch passed +1 javac 1m 51s the patch passed -1 checkstyle 1m 14s hbase-server: The patch generated 3 new + 3 unchanged - 0 fixed = 6 total (was 3) +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedjars 4m 22s patch has no errors when building our shaded downstream artifacts. +1 hadoopcheck 11m 51s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0. +1 findbugs 2m 49s the patch passed +1 javadoc 0m 41s the patch passed       Other Tests +1 unit 145m 16s hbase-server in the patch passed. +1 asflicense 0m 30s The patch does not generate ASF License warnings. 190m 27s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b JIRA Issue HBASE-21342 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12944909/HBASE-21342.005.patch Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile uname Linux 2f5b93405f3e 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 10:45:36 UTC 2018 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build@2/component/dev-support/hbase-personality.sh git revision master / dd474ef199 maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z) Default Java 1.8.0_181 findbugs v3.1.0-RC3 checkstyle https://builds.apache.org/job/PreCommit-HBASE-Build/14786/artifact/patchprocess/diff-checkstyle-hbase-server.txt Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14786/testReport/ Max. process+thread count 4679 (vs. ulimit of 10000) modules C: hbase-server U: hbase-server Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14786/console Powered by Apache Yetus 0.8.0 http://yetus.apache.org This message was automatically generated.
            yuzhihong@gmail.com Ted Yu added a comment -

            mdrob:
            Can you review the patch one more time ?

            Thanks

            yuzhihong@gmail.com Ted Yu added a comment - mdrob : Can you review the patch one more time ? Thanks
            mdrob Mike Drob added a comment -

            Thanks for the patch, mazhenlin! It's looking much better now, I think the structure is all there. Just a few more minor issues with implementation.

            +  ConcurrentHashMap<UserGroupInformation, Integer> ugiReferenceCounter;
            +  InternalObserverForFSOperation testObserver = null;
            +  void incrementUgiReference(UserGroupInformation ugi) {
            +  void decrementUgiReference(UserGroupInformation ugi) {
            +  boolean isUserReferenced(UserGroupInformation ugi) {
            

            should be private

            + ugiReferenceCounter = new ConcurrentHashMap();

            Needs generics.

            +  @VisibleForTesting
            +  interface InternalObserverForFSOperation {
            +    void afterFileSystemCreated(HRegion r) throws Exception;
            +  }
            
              @Override
              public void afterFileSystemCreated(HRegion r) throws Exception{
                if (r.getRegionInfo().containsRow(key3)) {
                  ealierBulkload.join(); /// wait util the other bulkload finished
                }
              }
            

            Is the exception throwing necessary? I can't tell if we actually use that to fail the test or not. If not, then the interface could be Consumer<HRegion> instead of declaring our own.

            +  void incrementUgiReference(UserGroupInformation ugi) {
            +    ugiReferenceCounter.merge(ugi, 1, new BiFunction<Integer, Integer, Integer>() {
            +      @Override
            +      public Integer apply(Integer integer, Integer integer2) {
            +        return ++integer;
            +      }
            +    });
            +  }
            

            Personal preference would be to use a lambda here, but that's strictly personal preference. Would be good practice to call those integer arguments something more descriptive than "int" and "int2" for folks unfamiliar with how merge works. Maybe "oldvalue" and "value"

            +  boolean isUserReferenced(UserGroupInformation ugi) {
            +    return ugiReferenceCounter.containsKey(ugi) && ugiReferenceCounter.get(ugi) > 0;
            +  }
            

            Contains is effectively a get under the hood, so better performance would be to do a single get and check against null and > 0.

            +            /// just for test
            

            Unnecessary comment.

            +    /// create table
            +    HTableDescriptor htd2 = new HTableDescriptor(TABLE);
            +    htd2.addFamily(new HColumnDescriptor(FAMILY));
            +    testUtil.getHBaseAdmin().createTable(htd2, Bytes.toByteArrays(SPLIT_ROWKEY));
            

            Please don't use deprecated APIs. There's a helper method on HBaseTestUtility for create table to make this easier. createTable(TableName, byte[] family, byte[][] splits)

            +    HTableDescriptor desc = testUtil.getAdmin().getTableDescriptor(TABLE);
            +    HColumnDescriptor family = desc.getFamily(FAMILY);
            

            Again, avoid deprecated APIs.

            +  class MyExceptionToAvoidRetry extends DoNotRetryIOException {
            

            Why is the original DoNotRetryIOException insufficient here? Probably best to add a comment.

            +    ealierBulkload = new Thread(new Runnable() {
            +      @Override
            +      public void run() {
            +        try {
            +          doBulkloadWithoutRetry(dir1);
            +        } catch (Exception e) {
            +          LOG.error("bulk load failed .",e);
            +        }
            +      }
            +    });
            

            If this bulk load fails, do we want that to fail the whole test? I would assume we do, you can look at https://github.com/apache/hbase/blob/master/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureNonce.java#L181-L182 for a pattern of how we do this in other places.

            +@Category({RegionServerTests.class, SmallTests.class})
            

            This starts a mini-cluster, so I think that would make it a medium test at a minimum.

            mdrob Mike Drob added a comment - Thanks for the patch, mazhenlin ! It's looking much better now, I think the structure is all there. Just a few more minor issues with implementation. + ConcurrentHashMap<UserGroupInformation, Integer > ugiReferenceCounter; + InternalObserverForFSOperation testObserver = null ; + void incrementUgiReference(UserGroupInformation ugi) { + void decrementUgiReference(UserGroupInformation ugi) { + boolean isUserReferenced(UserGroupInformation ugi) { should be private + ugiReferenceCounter = new ConcurrentHashMap(); Needs generics. + @VisibleForTesting + interface InternalObserverForFSOperation { + void afterFileSystemCreated(HRegion r) throws Exception; + } @Override public void afterFileSystemCreated(HRegion r) throws Exception{ if (r.getRegionInfo().containsRow(key3)) { ealierBulkload.join(); /// wait util the other bulkload finished } } Is the exception throwing necessary? I can't tell if we actually use that to fail the test or not. If not, then the interface could be Consumer<HRegion> instead of declaring our own. + void incrementUgiReference(UserGroupInformation ugi) { + ugiReferenceCounter.merge(ugi, 1, new BiFunction< Integer , Integer , Integer >() { + @Override + public Integer apply( Integer integer, Integer integer2) { + return ++integer; + } + }); + } Personal preference would be to use a lambda here, but that's strictly personal preference. Would be good practice to call those integer arguments something more descriptive than "int" and "int2" for folks unfamiliar with how merge works. Maybe "oldvalue" and "value" + boolean isUserReferenced(UserGroupInformation ugi) { + return ugiReferenceCounter.containsKey(ugi) && ugiReferenceCounter.get(ugi) > 0; + } Contains is effectively a get under the hood, so better performance would be to do a single get and check against null and > 0. + /// just for test Unnecessary comment. + /// create table + HTableDescriptor htd2 = new HTableDescriptor(TABLE); + htd2.addFamily( new HColumnDescriptor(FAMILY)); + testUtil.getHBaseAdmin().createTable(htd2, Bytes.toByteArrays(SPLIT_ROWKEY)); Please don't use deprecated APIs. There's a helper method on HBaseTestUtility for create table to make this easier. createTable(TableName, byte[] family, byte[][] splits) + HTableDescriptor desc = testUtil.getAdmin().getTableDescriptor(TABLE); + HColumnDescriptor family = desc.getFamily(FAMILY); Again, avoid deprecated APIs. + class MyExceptionToAvoidRetry extends DoNotRetryIOException { Why is the original DoNotRetryIOException insufficient here? Probably best to add a comment. + ealierBulkload = new Thread ( new Runnable () { + @Override + public void run() { + try { + doBulkloadWithoutRetry(dir1); + } catch (Exception e) { + LOG.error( "bulk load failed ." ,e); + } + } + }); If this bulk load fails, do we want that to fail the whole test? I would assume we do, you can look at https://github.com/apache/hbase/blob/master/hbase-procedure/src/test/java/org/apache/hadoop/hbase/procedure2/TestProcedureNonce.java#L181-L182 for a pattern of how we do this in other places. +@Category({RegionServerTests.class, SmallTests.class}) This starts a mini-cluster, so I think that would make it a medium test at a minimum.
            mdrob Mike Drob added a comment -

            Thanks for the quick turnaround!

            We have SecureBulkLoadManager and TestSecureBulkloadManager, capitalization should be consistent.

                final AtomicReference<Throwable> t1Exception = new AtomicReference();
                final AtomicReference<Throwable> t2Exception = new AtomicReference();
            

            This needs generics, my fault for pointing you at an example that didn't include them.

                Assert.assertEquals(null, t1Exception.get());
                Assert.assertEquals(null, t2Exception.get());
            

            Could be assertNull.

                try {
                  h.doBulkLoad(dir, testUtil.getAdmin(), connection.getTable(TABLE),
                      connection.getRegionLocator(TABLE));
                } catch (MyExceptionToAvoidRetry e) {
                }
            

            Please add a descriptive Assert.fail to the try block and a comment //expected to the catch.

            mdrob Mike Drob added a comment - Thanks for the quick turnaround! We have SecureBulkLoadManager and TestSecureBulkloadManager, capitalization should be consistent. final AtomicReference<Throwable> t1Exception = new AtomicReference(); final AtomicReference<Throwable> t2Exception = new AtomicReference(); This needs generics, my fault for pointing you at an example that didn't include them. Assert.assertEquals( null , t1Exception.get()); Assert.assertEquals( null , t2Exception.get()); Could be assertNull . try { h.doBulkLoad(dir, testUtil.getAdmin(), connection.getTable(TABLE), connection.getRegionLocator(TABLE)); } catch (MyExceptionToAvoidRetry e) { } Please add a descriptive Assert.fail to the try block and a comment //expected to the catch.
            mdrob Mike Drob added a comment -

            Also, if you generate the patch using, git format-patch instead of git diff then it will be easier for us to retain credit for your authorship in the commit.

            mdrob Mike Drob added a comment - Also, if you generate the patch using, git format-patch instead of git diff then it will be easier for us to retain credit for your authorship in the commit.
            hadoopqa Hadoop QA added a comment -
            +1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 11s Docker mode activated.
                  Prechecks
            +1 hbaseanti 0m 0s Patch does not have any anti-patterns.
            +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.
                  master Compile Tests
            +1 mvninstall 6m 49s master passed
            +1 compile 2m 7s master passed
            +1 checkstyle 1m 19s master passed
            +1 shadedjars 5m 17s branch has no errors when building our shaded downstream artifacts.
            +1 findbugs 2m 23s master passed
            +1 javadoc 0m 37s master passed
                  Patch Compile Tests
            +1 mvninstall 6m 14s the patch passed
            +1 compile 2m 14s the patch passed
            +1 javac 2m 14s the patch passed
            +1 checkstyle 1m 18s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 shadedjars 4m 39s patch has no errors when building our shaded downstream artifacts.
            +1 hadoopcheck 13m 2s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
            +1 findbugs 2m 35s the patch passed
            +1 javadoc 0m 39s the patch passed
                  Other Tests
            +1 unit 137m 38s hbase-server in the patch passed.
            +1 asflicense 0m 22s The patch does not generate ASF License warnings.
            187m 54s



            Subsystem Report/Notes
            Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b
            JIRA Issue HBASE-21342
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12945182/HBASE-21342.006.patch
            Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
            uname Linux 0cfa10fc6521 4.4.0-134-generic #160~14.04.1-Ubuntu SMP Fri Aug 17 11:07:07 UTC 2018 x86_64 GNU/Linux
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh
            git revision master / 3b68e5393e
            maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
            Default Java 1.8.0_181
            findbugs v3.1.0-RC3
            Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14820/testReport/
            Max. process+thread count 4843 (vs. ulimit of 10000)
            modules C: hbase-server U: hbase-server
            Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14820/console
            Powered by Apache Yetus 0.8.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated.       Prechecks +1 hbaseanti 0m 0s Patch does not have any anti-patterns. +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.       master Compile Tests +1 mvninstall 6m 49s master passed +1 compile 2m 7s master passed +1 checkstyle 1m 19s master passed +1 shadedjars 5m 17s branch has no errors when building our shaded downstream artifacts. +1 findbugs 2m 23s master passed +1 javadoc 0m 37s master passed       Patch Compile Tests +1 mvninstall 6m 14s the patch passed +1 compile 2m 14s the patch passed +1 javac 2m 14s the patch passed +1 checkstyle 1m 18s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedjars 4m 39s patch has no errors when building our shaded downstream artifacts. +1 hadoopcheck 13m 2s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0. +1 findbugs 2m 35s the patch passed +1 javadoc 0m 39s the patch passed       Other Tests +1 unit 137m 38s hbase-server in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 187m 54s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b JIRA Issue HBASE-21342 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12945182/HBASE-21342.006.patch Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile uname Linux 0cfa10fc6521 4.4.0-134-generic #160~14.04.1-Ubuntu SMP Fri Aug 17 11:07:07 UTC 2018 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh git revision master / 3b68e5393e maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z) Default Java 1.8.0_181 findbugs v3.1.0-RC3 Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14820/testReport/ Max. process+thread count 4843 (vs. ulimit of 10000) modules C: hbase-server U: hbase-server Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14820/console Powered by Apache Yetus 0.8.0 http://yetus.apache.org This message was automatically generated.
            hadoopqa Hadoop QA added a comment -
            +1 overall



            Vote Subsystem Runtime Comment
            0 reexec 0m 11s Docker mode activated.
                  Prechecks
            +1 hbaseanti 0m 0s Patch does not have any anti-patterns.
            +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.
                  master Compile Tests
            +1 mvninstall 5m 4s master passed
            +1 compile 1m 45s master passed
            +1 checkstyle 1m 10s master passed
            +1 shadedjars 4m 12s branch has no errors when building our shaded downstream artifacts.
            +1 findbugs 1m 58s master passed
            +1 javadoc 0m 30s master passed
                  Patch Compile Tests
            +1 mvninstall 5m 6s the patch passed
            +1 compile 1m 49s the patch passed
            +1 javac 1m 49s the patch passed
            +1 checkstyle 1m 12s the patch passed
            +1 whitespace 0m 0s The patch has no whitespace issues.
            +1 shadedjars 4m 15s patch has no errors when building our shaded downstream artifacts.
            +1 hadoopcheck 10m 33s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
            +1 findbugs 2m 5s the patch passed
            +1 javadoc 0m 30s the patch passed
                  Other Tests
            +1 unit 130m 41s hbase-server in the patch passed.
            +1 asflicense 0m 22s The patch does not generate ASF License warnings.
            171m 54s



            Subsystem Report/Notes
            Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b
            JIRA Issue HBASE-21342
            JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12945244/HBASE-21342.007.patch
            Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
            uname Linux 937d90be826e 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 10:45:36 UTC 2018 x86_64 GNU/Linux
            Build tool maven
            Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh
            git revision master / 3b68e5393e
            maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
            Default Java 1.8.0_181
            findbugs v3.1.0-RC3
            Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14825/testReport/
            Max. process+thread count 4698 (vs. ulimit of 10000)
            modules C: hbase-server U: hbase-server
            Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14825/console
            Powered by Apache Yetus 0.8.0 http://yetus.apache.org

            This message was automatically generated.

            hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated.       Prechecks +1 hbaseanti 0m 0s Patch does not have any anti-patterns. +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.       master Compile Tests +1 mvninstall 5m 4s master passed +1 compile 1m 45s master passed +1 checkstyle 1m 10s master passed +1 shadedjars 4m 12s branch has no errors when building our shaded downstream artifacts. +1 findbugs 1m 58s master passed +1 javadoc 0m 30s master passed       Patch Compile Tests +1 mvninstall 5m 6s the patch passed +1 compile 1m 49s the patch passed +1 javac 1m 49s the patch passed +1 checkstyle 1m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedjars 4m 15s patch has no errors when building our shaded downstream artifacts. +1 hadoopcheck 10m 33s Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0. +1 findbugs 2m 5s the patch passed +1 javadoc 0m 30s the patch passed       Other Tests +1 unit 130m 41s hbase-server in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 171m 54s Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:b002b0b JIRA Issue HBASE-21342 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12945244/HBASE-21342.007.patch Optional Tests dupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile uname Linux 937d90be826e 3.13.0-143-generic #192-Ubuntu SMP Tue Feb 27 10:45:36 UTC 2018 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build/component/dev-support/hbase-personality.sh git revision master / 3b68e5393e maven version: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z) Default Java 1.8.0_181 findbugs v3.1.0-RC3 Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/14825/testReport/ Max. process+thread count 4698 (vs. ulimit of 10000) modules C: hbase-server U: hbase-server Console output https://builds.apache.org/job/PreCommit-HBASE-Build/14825/console Powered by Apache Yetus 0.8.0 http://yetus.apache.org This message was automatically generated.
            mdrob Mike Drob added a comment -

            v7 LGTM. You want to commit this one, Ted?

            mdrob Mike Drob added a comment - v7 LGTM. You want to commit this one, Ted?
            yuzhihong@gmail.com Ted Yu added a comment -

            Mike:
            Please go ahead.

            Thanks

            yuzhihong@gmail.com Ted Yu added a comment - Mike: Please go ahead. Thanks

            Why did we drop all of the branch-1 fix versions? Is this not an issue there?

            apurtell Andrew Kyle Purtell added a comment - Why did we drop all of the branch-1 fix versions? Is this not an issue there?
            mdrob Mike Drob added a comment -

            I didn't drop the branch-1 versions, they were never in the fix version list. They're in affects version still.

            Paused my backports to consult with Stack offline about wether it's safe to commit to branch-2.1 right now, since I know he's prepping for a release. Will likely spin off separate backport issues.

            mdrob Mike Drob added a comment - I didn't drop the branch-1 versions, they were never in the fix version list. They're in affects version still. Paused my backports to consult with Stack offline about wether it's safe to commit to branch-2.1 right now, since I know he's prepping for a release. Will likely spin off separate backport issues.

            The affects versions are set. Shouldn’t the fix versions be the same if that code is similarly affected? Are we leaving known bugs in branch-1 unfixed by policy now? Separate back port JIRA is better than nothing but not much

            apurtell Andrew Kyle Purtell added a comment - The affects versions are set. Shouldn’t the fix versions be the same if that code is similarly affected? Are we leaving known bugs in branch-1 unfixed by policy now? Separate back port JIRA is better than nothing but not much
            mdrob Mike Drob added a comment -

            I'm not trying to ignore any versions or leave anything unfixed. I'm not trying to make policy.

            The fix versions are the set of versions that I had already done backports and pushed to. I hadn't pushed code to branch-1 or branch-2.0 yet, so they weren't included. I was using fix version in Jira as descriptive, not prescriptive. I wanted to be able to close the issue so that the RM could generate release notes if needed, and then expected to continue work in a separate backport issue.

            I have pushed this to branch-2.0+

            There is a conflict cherry-picking to branch-1 that I don't have time to resolve today. I will open a backport Jira to not conflict with the release notes for Stack if he cuts a 2.1.1 tonight.

            mdrob Mike Drob added a comment - I'm not trying to ignore any versions or leave anything unfixed. I'm not trying to make policy. The fix versions are the set of versions that I had already done backports and pushed to. I hadn't pushed code to branch-1 or branch-2.0 yet, so they weren't included. I was using fix version in Jira as descriptive, not prescriptive. I wanted to be able to close the issue so that the RM could generate release notes if needed, and then expected to continue work in a separate backport issue. I have pushed this to branch-2.0+ There is a conflict cherry-picking to branch-1 that I don't have time to resolve today. I will open a backport Jira to not conflict with the release notes for Stack if he cuts a 2.1.1 tonight.
            hudson Hudson added a comment -

            Results for branch branch-2.1
            build #522 on builds.a.o: +1 overall


            details (if available):

            +1 general checks
            – For more information see general report

            +1 jdk8 hadoop2 checks
            – For more information see jdk8 (hadoop2) report

            +1 jdk8 hadoop3 checks
            – For more information see jdk8 (hadoop3) report

            +1 source release artifact
            – See build output for details.

            +1 client integration test

            hudson Hudson added a comment - Results for branch branch-2.1 build #522 on builds.a.o : +1 overall details (if available): +1 general checks – For more information see general report +1 jdk8 hadoop2 checks – For more information see jdk8 (hadoop2) report +1 jdk8 hadoop3 checks – For more information see jdk8 (hadoop3) report +1 source release artifact – See build output for details. +1 client integration test
            hudson Hudson added a comment -

            Results for branch branch-2.0
            build #1004 on builds.a.o: -1 overall


            details (if available):

            +1 general checks
            – For more information see general report

            -1 jdk8 hadoop2 checks
            – For more information see jdk8 (hadoop2) report

            +1 jdk8 hadoop3 checks
            – For more information see jdk8 (hadoop3) report

            +1 source release artifact
            – See build output for details.

            hudson Hudson added a comment - Results for branch branch-2.0 build #1004 on builds.a.o : -1 overall details (if available): +1 general checks – For more information see general report -1 jdk8 hadoop2 checks – For more information see jdk8 (hadoop2) report +1 jdk8 hadoop3 checks – For more information see jdk8 (hadoop3) report +1 source release artifact – See build output for details.
            hudson Hudson added a comment -

            Results for branch branch-2
            build #1435 on builds.a.o: -1 overall


            details (if available):

            +1 general checks
            – For more information see general report

            -1 jdk8 hadoop2 checks
            – For more information see jdk8 (hadoop2) report

            +1 jdk8 hadoop3 checks
            – For more information see jdk8 (hadoop3) report

            +1 source release artifact
            – See build output for details.

            +1 client integration test

            hudson Hudson added a comment - Results for branch branch-2 build #1435 on builds.a.o : -1 overall details (if available): +1 general checks – For more information see general report -1 jdk8 hadoop2 checks – For more information see jdk8 (hadoop2) report +1 jdk8 hadoop3 checks – For more information see jdk8 (hadoop3) report +1 source release artifact – See build output for details. +1 client integration test
            hudson Hudson added a comment -

            Results for branch master
            build #565 on builds.a.o: -1 overall


            details (if available):

            +1 general checks
            – For more information see general report

            -1 jdk8 hadoop2 checks
            – For more information see jdk8 (hadoop2) report

            -1 jdk8 hadoop3 checks
            – For more information see jdk8 (hadoop3) report

            +1 source release artifact
            – See build output for details.

            +1 client integration test

            hudson Hudson added a comment - Results for branch master build #565 on builds.a.o : -1 overall details (if available): +1 general checks – For more information see general report -1 jdk8 hadoop2 checks – For more information see jdk8 (hadoop2) report -1 jdk8 hadoop3 checks – For more information see jdk8 (hadoop3) report +1 source release artifact – See build output for details. +1 client integration test
            hudson Hudson added a comment -

            SUCCESS: Integrated in Jenkins build HBase-1.3-IT #516 (See https://builds.apache.org/job/HBase-1.3-IT/516/)
            HBASE-21374 Backport HBASE-21342 to branch-1 (apurtell: https://github.com/apache/hbase/commit/25d0c3ec7f80f51e814ed53caa9509fe60a4a8cf)

            • (edit) hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSecureBulkLoadEndpoint.java
            • (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SecureBulkLoadEndpoint.java
            hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build HBase-1.3-IT #516 (See https://builds.apache.org/job/HBase-1.3-IT/516/ ) HBASE-21374 Backport HBASE-21342 to branch-1 (apurtell: https://github.com/apache/hbase/commit/25d0c3ec7f80f51e814ed53caa9509fe60a4a8cf ) (edit) hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSecureBulkLoadEndpoint.java (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SecureBulkLoadEndpoint.java
            hudson Hudson added a comment -

            Results for branch branch-1.3
            build #606 on builds.a.o: -1 overall


            details (if available):

            +1 general checks
            – For more information see general report

            -1 jdk7 checks
            – For more information see jdk7 report

            -1 jdk8 hadoop2 checks
            – For more information see jdk8 (hadoop2) report

            +1 source release artifact
            – See build output for details.

            hudson Hudson added a comment - Results for branch branch-1.3 build #606 on builds.a.o : -1 overall details (if available): +1 general checks – For more information see general report -1 jdk7 checks – For more information see jdk7 report -1 jdk8 hadoop2 checks – For more information see jdk8 (hadoop2) report +1 source release artifact – See build output for details.
            hudson Hudson added a comment -

            Results for branch branch-1.4
            build #623 on builds.a.o: -1 overall


            details (if available):

            -1 general checks
            – For more information see general report

            -1 jdk7 checks
            – For more information see jdk7 report

            -1 jdk8 hadoop2 checks
            – For more information see jdk8 (hadoop2) report

            +1 source release artifact
            – See build output for details.

            hudson Hudson added a comment - Results for branch branch-1.4 build #623 on builds.a.o : -1 overall details (if available): -1 general checks – For more information see general report -1 jdk7 checks – For more information see jdk7 report -1 jdk8 hadoop2 checks – For more information see jdk8 (hadoop2) report +1 source release artifact – See build output for details.
            hudson Hudson added a comment -

            Results for branch branch-1
            build #627 on builds.a.o: -1 overall


            details (if available):

            -1 general checks
            – For more information see general report

            -1 jdk7 checks
            – For more information see jdk7 report

            -1 jdk8 hadoop2 checks
            – For more information see jdk8 (hadoop2) report

            -1 source release artifact
            – See build output for details.

            hudson Hudson added a comment - Results for branch branch-1 build #627 on builds.a.o : -1 overall details (if available): -1 general checks – For more information see general report -1 jdk7 checks – For more information see jdk7 report -1 jdk8 hadoop2 checks – For more information see jdk8 (hadoop2) report -1 source release artifact – See build output for details.
            hudson Hudson added a comment -

            SUCCESS: Integrated in Jenkins build HBase-1.2-IT #1202 (See https://builds.apache.org/job/HBase-1.2-IT/1202/)
            HBASE-21374 Backport HBASE-21342 to branch-1 (busbey: https://github.com/apache/hbase/commit/f7b51ccdf7414bc1deada02e3add556dc76e97be)

            • (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
            • (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SecureBulkLoadEndpoint.java
            • (edit) hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSecureBulkLoadEndpoint.java
            hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build HBase-1.2-IT #1202 (See https://builds.apache.org/job/HBase-1.2-IT/1202/ ) HBASE-21374 Backport HBASE-21342 to branch-1 (busbey: https://github.com/apache/hbase/commit/f7b51ccdf7414bc1deada02e3add556dc76e97be ) (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SecureBulkLoadEndpoint.java (edit) hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSecureBulkLoadEndpoint.java
            hudson Hudson added a comment -

            Results for branch branch-1.2
            build #651 on builds.a.o: -1 overall


            details (if available):

            +1 general checks
            – For more information see general report

            +1 jdk7 checks
            – For more information see jdk7 report

            -1 jdk8 hadoop2 checks
            – For more information see jdk8 (hadoop2) report

            +1 source release artifact
            – See build output for details.

            hudson Hudson added a comment - Results for branch branch-1.2 build #651 on builds.a.o : -1 overall details (if available): +1 general checks – For more information see general report +1 jdk7 checks – For more information see jdk7 report -1 jdk8 hadoop2 checks – For more information see jdk8 (hadoop2) report +1 source release artifact – See build output for details.

            People

              mazhenlin mazhenlin
              mazhenlin mazhenlin
              Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: