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
-
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
Attachments
- 21342.v1.txt
- 12 kB
- Ted Yu
- HBASE-21342.002.patch
- 12 kB
- mazhenlin
- HBASE-21342.003.patch
- 13 kB
- mazhenlin
- HBASE-21342.004.patch
- 14 kB
- mazhenlin
- HBASE-21342.005.patch
- 14 kB
- mazhenlin
- HBASE-21342.006.patch
- 15 kB
- mazhenlin
- HBASE-21342.007.patch
- 16 kB
- mazhenlin
- race.patch
- 12 kB
- mazhenlin
Issue Links
- breaks
-
HBASE-21884 Fix box/unbox findbugs warning in secure bulk load
- Resolved
- relates to
-
HBASE-21374 Backport HBASE-21342 to branch-1
- Resolved
-
HBASE-15291 FileSystem not closed in secure bulkLoad
- Resolved
Activity
+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);
mdrob Thank you. I will submit my patch tomorrow. Right now I have not figured out how to submit
+ 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?
Added ClassRule to TestSecureBulkloadManager.
Let's see what QA bot says.
Zhenlin:
Please add Apache license header to TestSecureBulkloadManager in your next patch.
-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 | |
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.
Can you change the name of the test table to reflect the nature of the test ?
Thanks
-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 | |
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.
+ 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.
-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 | |
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.
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.
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.
-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 | |
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.
-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 | |
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.
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.
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.
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.
+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 | |
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.
+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 | |
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.
Why did we drop all of the branch-1 fix versions? Is this not an issue there?
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
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.
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
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.
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
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
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
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.
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.
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.
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
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.
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!