From f24879bd55a6c0a99c4fe396d4b2dad2c3a7c296 Mon Sep 17 00:00:00 2001 From: Mike Drob Date: Mon, 6 Nov 2017 21:16:48 -0600 Subject: [PATCH] HBASE-19195 error-prone fixes for client, mr, and server --- .../org/apache/hadoop/hbase/filter/FilterListWithAND.java | 4 +++- .../hadoop/hbase/mapreduce/TableInputFormatBase.java | 4 ++-- .../org/apache/hadoop/hbase/PerformanceEvaluation.java | 2 +- .../org/apache/hadoop/hbase/mapred/TestSplitTable.java | 8 ++++---- .../org/apache/hadoop/hbase/mapreduce/TestImportTsv.java | 3 ++- .../mapreduce/TsvImporterCustomTestMapperForOprAttr.java | 9 +++++++-- .../apache/hadoop/hbase/client/TestFromClientSide3.java | 15 ++++++++++----- .../hadoop/hbase/regionserver/TestMajorCompaction.java | 1 + 8 files changed, 30 insertions(+), 16 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java index edb5d3e27b..25376cbc29 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FilterListWithAND.java @@ -90,10 +90,12 @@ public class FilterListWithAND extends FilterListBase { * code of current sub-filter. */ private ReturnCode mergeReturnCode(ReturnCode rc, ReturnCode localRC) { - if (rc == ReturnCode.SEEK_NEXT_USING_HINT || localRC == ReturnCode.SEEK_NEXT_USING_HINT) { + if (rc == ReturnCode.SEEK_NEXT_USING_HINT) { return ReturnCode.SEEK_NEXT_USING_HINT; } switch (localRC) { + case SEEK_NEXT_USING_HINT: + return ReturnCode.SEEK_NEXT_USING_HINT; case INCLUDE: return rc; case INCLUDE_AND_NEXT_COL: diff --git a/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableInputFormatBase.java b/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableInputFormatBase.java index e7a65e8591..4d3e65cfe2 100644 --- a/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableInputFormatBase.java +++ b/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/TableInputFormatBase.java @@ -268,8 +268,8 @@ public abstract class TableInputFormatBase } //The default value of "hbase.mapreduce.input.autobalance" is false. - if (context.getConfiguration().getBoolean(MAPREDUCE_INPUT_AUTOBALANCE, false) != false) { - long maxAveRegionSize = context.getConfiguration().getInt(MAX_AVERAGE_REGION_SIZE, 8*1073741824); + if (context.getConfiguration().getBoolean(MAPREDUCE_INPUT_AUTOBALANCE, false)) { + long maxAveRegionSize = context.getConfiguration().getLong(MAX_AVERAGE_REGION_SIZE, 8L*1073741824); //8GB return calculateAutoBalancedSplits(splits, maxAveRegionSize); } diff --git a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java index 2bf94f41df..dd793f2534 100644 --- a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java +++ b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/PerformanceEvaluation.java @@ -1068,7 +1068,7 @@ public class PerformanceEvaluation extends Configured implements Tool { } int getValueLength(final Random r) { - if (this.opts.isValueRandom()) return Math.abs(r.nextInt() % opts.valueSize); + if (this.opts.isValueRandom()) return r.nextInt(opts.valueSize); else if (this.opts.isValueZipf()) return Math.abs(this.zipf.nextInt()); else return opts.valueSize; } diff --git a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapred/TestSplitTable.java b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapred/TestSplitTable.java index 2655ac215f..2897e7bee5 100644 --- a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapred/TestSplitTable.java +++ b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapred/TestSplitTable.java @@ -38,7 +38,7 @@ public class TestSplitTable { public TestName name = new TestName(); @Test - @SuppressWarnings("deprecation") + @SuppressWarnings({"deprecation", "SelfComparison"}) public void testSplitTableCompareTo() { TableSplit aTableSplit = new TableSplit(Bytes.toBytes("tableA"), Bytes.toBytes("aaa"), Bytes.toBytes("ddd"), "locationA"); @@ -49,9 +49,9 @@ public class TestSplitTable { TableSplit cTableSplit = new TableSplit(Bytes.toBytes("tableA"), Bytes.toBytes("lll"), Bytes.toBytes("zzz"), "locationA"); - assertTrue(aTableSplit.compareTo(aTableSplit) == 0); - assertTrue(bTableSplit.compareTo(bTableSplit) == 0); - assertTrue(cTableSplit.compareTo(cTableSplit) == 0); + assertEquals(0, aTableSplit.compareTo(aTableSplit)); + assertEquals(0, bTableSplit.compareTo(bTableSplit)); + assertEquals(0, cTableSplit.compareTo(cTableSplit)); assertTrue(aTableSplit.compareTo(bTableSplit) < 0); assertTrue(bTableSplit.compareTo(aTableSplit) > 0); diff --git a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTsv.java b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTsv.java index 6ad7694356..f6fcfa38ec 100644 --- a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTsv.java +++ b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportTsv.java @@ -23,6 +23,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.io.IOException; +import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -430,7 +431,7 @@ public class TestImportTsv implements Configurable { // run the import Tool tool = new ImportTsv(); - LOG.debug("Running ImportTsv with arguments: " + argsArray); + LOG.debug("Running ImportTsv with arguments: " + Arrays.toString(argsArray)); assertEquals(0, ToolRunner.run(conf, tool, argsArray)); // Perform basic validation. If the input args did not include diff --git a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TsvImporterCustomTestMapperForOprAttr.java b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TsvImporterCustomTestMapperForOprAttr.java index a9da98b531..850d4abac8 100644 --- a/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TsvImporterCustomTestMapperForOprAttr.java +++ b/hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TsvImporterCustomTestMapperForOprAttr.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.mapreduce; import java.io.IOException; +import java.util.Arrays; import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.client.Put; @@ -43,10 +44,10 @@ public class TsvImporterCustomTestMapperForOprAttr extends TsvImporterMapper { for (String attr : attributes) { String[] split = attr.split(ImportTsv.DEFAULT_ATTRIBUTES_SEPERATOR); if (split == null || split.length <= 1) { - throw new BadTsvLineException("Invalid attributes seperator specified" + attributes); + throw new BadTsvLineException(msg(attributes)); } else { if (split[0].length() <= 0 || split[1].length() <= 0) { - throw new BadTsvLineException("Invalid attributes seperator specified" + attributes); + throw new BadTsvLineException(msg(attributes)); } put.setAttribute(split[0], Bytes.toBytes(split[1])); } @@ -54,4 +55,8 @@ public class TsvImporterCustomTestMapperForOprAttr extends TsvImporterMapper { } put.add(kv); } + + private String msg(Object[] attributes) { + return "Invalid attributes separator specified: " + Arrays.toString(attributes); + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java index 0a3b75ab1f..b90817349b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java @@ -361,7 +361,7 @@ public class TestFromClientSide3 { break; } } catch (Exception e) { - LOG.debug("Waiting for region to come online: " + regionName); + LOG.debug("Waiting for region to come online: " + Bytes.toString(regionName)); } Thread.sleep(40); } @@ -478,6 +478,7 @@ public class TestFromClientSide3 { assertEquals(exist, true); } + @Test public void testHTableExistsMethodSingleRegionMultipleGets() throws Exception { Table table = TEST_UTIL.createTable(TableName.valueOf( name.getMethodName()), new byte[][] { FAMILY }); @@ -749,7 +750,7 @@ public class TestFromClientSide3 { try (Table table = con.getTable(tableName)) { table.append(append); fail("The APPEND should fail because the target lock is blocked by previous put"); - } catch (Throwable ex) { + } catch (Exception ex) { } }); appendService.shutdown(); @@ -802,6 +803,7 @@ public class TestFromClientSide3 { }); ExecutorService cpService = Executors.newSingleThreadExecutor(); cpService.execute(() -> { + boolean threw; Put put1 = new Put(row); Put put2 = new Put(rowLocked); put1.addColumn(FAMILY, QUALIFIER, value1); @@ -823,10 +825,13 @@ public class TestFromClientSide3 { exe.mutateRows(controller, request, rpcCallback); return rpcCallback.get(); }); + threw = false; + } catch (Throwable ex) { + threw = true; + } + if (!threw) { + // Can't call fail() earlier because the catch would eat it. fail("This cp should fail because the target lock is blocked by previous put"); - } catch (Throwable ex) { - // TODO!!!! Is this right? It catches everything including the above fail - // if it happens (which it seems too....) } }); cpService.shutdown(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMajorCompaction.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMajorCompaction.java index 2a556d7a52..40f513562d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMajorCompaction.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMajorCompaction.java @@ -450,6 +450,7 @@ public class TestMajorCompaction { * basically works. * @throws IOException */ + @Test public void testMajorCompactingToNoOutputWithReverseScan() throws IOException { createStoreFile(r); for (int i = 0; i < compactionThreshold; i++) { -- 2.14.1