From 24d82195cb82bd90755408b85d70c57cf4b3ff2f Mon Sep 17 00:00:00 2001 From: Chia-Ping Tsai Date: Tue, 28 Nov 2017 03:46:14 +0800 Subject: [PATCH] HBASE-19348 Fix error-prone errors for branch-1 Signed-off-by: Andrew Purtell --- ...sterAnnotationReadingPriorityFunction.java | 10 +- .../procedure/ServerCrashProcedure.java | 4 +- .../master/snapshot/CloneSnapshotHandler.java | 1 - .../org/apache/hadoop/hbase/TestCompare.java | 1 + .../hadoop/hbase/TestHRegionLocation.java | 1 + .../hbase/TestIPv6NIOServerSocketChannel.java | 2 +- .../apache/hadoop/hbase/TestZooKeeper.java | 4 +- .../hadoop/hbase/client/TestMetaScanner.java | 1 + .../client/TestScannersFromClientSide2.java | 1 + .../coprocessor/TestCoprocessorInterface.java | 4 +- .../hadoop/hbase/filter/TestFilter.java | 1 + ...TestMasterOperationsForRegionReplicas.java | 3 +- .../TestMasterProcedureScheduler.java | 2 + .../TestWALProcedureStoreOnHDFS.java | 143 ++++++++---------- 14 files changed, 86 insertions(+), 92 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterAnnotationReadingPriorityFunction.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterAnnotationReadingPriorityFunction.java index 1e6dadecca7..dc5d8241c2b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterAnnotationReadingPriorityFunction.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterAnnotationReadingPriorityFunction.java @@ -70,12 +70,10 @@ public class MasterAnnotationReadingPriorityFunction extends AnnotationReadingPr RegionServerStatusProtos.ReportRegionStateTransitionRequest tRequest = (RegionServerStatusProtos.ReportRegionStateTransitionRequest) param; for (RegionServerStatusProtos.RegionStateTransition rst : tRequest.getTransitionList()) { - if (rst.getRegionInfoList() != null) { - for (HBaseProtos.RegionInfo info : rst.getRegionInfoList()) { - TableName tn = ProtobufUtil.toTableName(info.getTableName()); - if (tn.isSystemTable()) { - return HConstants.SYSTEMTABLE_QOS; - } + for (HBaseProtos.RegionInfo info : rst.getRegionInfoList()) { + TableName tn = ProtobufUtil.toTableName(info.getTableName()); + if (tn.isSystemTable()) { + return HConstants.SYSTEMTABLE_QOS; } } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java index 3463000150e..1fbc428bd79 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ServerCrashProcedure.java @@ -183,8 +183,8 @@ implements ServerProcedureInterface { LOG.trace(state); } // Keep running count of cycles - if (state.ordinal() != this.previousState) { - this.previousState = state.ordinal(); + if (state.getNumber() != this.previousState) { + this.previousState = state.getNumber(); this.cycles = 0; } else { this.cycles++; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/CloneSnapshotHandler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/CloneSnapshotHandler.java index 6f8bcd4db87..f4a6c95fe2f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/CloneSnapshotHandler.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/CloneSnapshotHandler.java @@ -123,7 +123,6 @@ public class CloneSnapshotHandler extends CreateTableHandler implements Snapshot // Clone acl of snapshot into newly created table. if (restoreAcl && snapshot.hasUsersAndPermissions() - && snapshot.getUsersAndPermissions() != null && SnapshotDescriptionUtils.isSecurityAvailable(conf)) { RestoreSnapshotHelper.restoreSnapshotACL(snapshot, tableName, conf); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestCompare.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestCompare.java index 4b420288fde..a69df34a72d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestCompare.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestCompare.java @@ -33,6 +33,7 @@ public class TestCompare extends TestCase { /** * Sort of HRegionInfo. */ + @SuppressWarnings({"SelfComparison"}) public void testHRegionInfo() { HRegionInfo a = new HRegionInfo(TableName.valueOf("a"), null, null); HRegionInfo b = new HRegionInfo(TableName.valueOf("b"), null, null); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestHRegionLocation.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestHRegionLocation.java index f6488d048b9..3d6ffb900c0 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestHRegionLocation.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestHRegionLocation.java @@ -62,6 +62,7 @@ public class TestHRegionLocation { System.out.println(hrl1.toString()); } + @SuppressWarnings("SelfComparison") @Test public void testCompareTo() { ServerName hsa1 = ServerName.valueOf("localhost", 1234, -1L); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestIPv6NIOServerSocketChannel.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestIPv6NIOServerSocketChannel.java index 3dc28711c0b..6fd2a0eaf30 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestIPv6NIOServerSocketChannel.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestIPv6NIOServerSocketChannel.java @@ -123,7 +123,7 @@ public class TestIPv6NIOServerSocketChannel { //On Windows JDK6, we will get expected exception: //java.net.SocketException: Address family not supported by protocol family //or java.net.SocketException: Protocol family not supported - Assert.assertFalse(ex.getClass().isInstance(BindException.class)); + Assert.assertFalse(ex instanceof BindException); Assert.assertTrue(ex.getMessage().toLowerCase(Locale.ROOT).contains("protocol family")); LOG.info("Received expected exception:"); LOG.info(ex); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java index 3441aa629bb..9070033661f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java @@ -69,6 +69,7 @@ import org.junit.AfterClass; import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -141,7 +142,8 @@ public class TestZooKeeper { * @throws InterruptedException */ // fails frequently, disabled for now, see HBASE-6406 - //@Test + @Ignore + @Test public void testClientSessionExpired() throws Exception { Configuration c = new Configuration(TEST_UTIL.getConfiguration()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaScanner.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaScanner.java index bead7e93d16..bca8cf3a1bb 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaScanner.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestMetaScanner.java @@ -54,6 +54,7 @@ public class TestMetaScanner { private final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); private Connection connection; + @SuppressWarnings("JUnit4SetUpNotRun") public void setUp() throws Exception { TEST_UTIL.startMiniCluster(1); this.connection = TEST_UTIL.getConnection(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScannersFromClientSide2.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScannersFromClientSide2.java index 4da94f25677..ff57ca0aefd 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScannersFromClientSide2.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScannersFromClientSide2.java @@ -217,6 +217,7 @@ public class TestScannersFromClientSide2 { testScan(456, false, 678, false, 200); } + @Test public void testReversedScanWithLimit() throws Exception { testReversedScan(998, true, 1, false, 900); // from last region to first region testReversedScan(543, true, 321, true, 100); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java index 32d6af4d426..906c103645b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestCoprocessorInterface.java @@ -272,9 +272,7 @@ public class TestCoprocessorInterface { @Override public void preGetOp(final ObserverContext e, final Get get, final List results) throws IOException { - if (1/0 == 1) { - e.complete(); - } + throw new RuntimeException(); } Map getSharedData() { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java index b8883bc4fd3..50532c0800f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/filter/TestFilter.java @@ -2049,6 +2049,7 @@ public class TestFilter { } } + @Test public void testNestedFilterListWithSCVF() throws IOException { byte[] columnStatus = Bytes.toBytes("S"); HTableDescriptor htd = new HTableDescriptor(TableName.valueOf("testNestedFilterListWithSCVF")); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterOperationsForRegionReplicas.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterOperationsForRegionReplicas.java index 2112be74dee..75f52436727 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterOperationsForRegionReplicas.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterOperationsForRegionReplicas.java @@ -55,6 +55,7 @@ import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.util.Bytes; import org.junit.AfterClass; import org.junit.BeforeClass; +import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -232,7 +233,7 @@ public class TestMasterOperationsForRegionReplicas { } } - //@Test (TODO: enable when we have support for alter_table- HBASE-10361). + @Test public void testIncompleteMetaTableReplicaInformation() throws Exception { final TableName table = TableName.valueOf("fooTableTest1"); final int numRegions = 3; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java index 2b594f42395..d3d9b520c20 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterProcedureScheduler.java @@ -488,6 +488,8 @@ public class TestMasterProcedureScheduler { case READ: queue.releaseTableSharedLock(proc, getTableName(proc)); break; + default: + break; } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestWALProcedureStoreOnHDFS.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestWALProcedureStoreOnHDFS.java index 8a9315133c6..f05b5880ee6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestWALProcedureStoreOnHDFS.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestWALProcedureStoreOnHDFS.java @@ -83,7 +83,9 @@ public class TestWALProcedureStoreOnHDFS { } }; - private static void initConfig(Configuration conf) { + @Before + public void initConfig() { + Configuration conf = UTIL.getConfiguration(); conf.setInt("dfs.replication", 3); conf.setInt("dfs.namenode.replication.min", 3); @@ -93,7 +95,8 @@ public class TestWALProcedureStoreOnHDFS { conf.setInt(WALProcedureStore.MAX_SYNC_FAILURE_ROLL_CONF_KEY, 10); } - public void setup() throws Exception { + // No @Before because some tests need to do additional config first + private void setup() throws Exception { MiniDFSCluster dfs = UTIL.startMiniDFSCluster(3); Path logDir = new Path(new Path(dfs.getFileSystem().getUri()), "/test-logs"); @@ -103,6 +106,7 @@ public class TestWALProcedureStoreOnHDFS { store.recoverLease(); } + @After public void tearDown() throws Exception { store.stop(false); UTIL.getDFSCluster().getFileSystem().delete(store.getWALDir(), true); @@ -116,102 +120,87 @@ public class TestWALProcedureStoreOnHDFS { @Test(timeout=60000, expected=RuntimeException.class) public void testWalAbortOnLowReplication() throws Exception { - initConfig(UTIL.getConfiguration()); setup(); - try { - assertEquals(3, UTIL.getDFSCluster().getDataNodes().size()); + assertEquals(3, UTIL.getDFSCluster().getDataNodes().size()); - LOG.info("Stop DataNode"); - UTIL.getDFSCluster().stopDataNode(0); + LOG.info("Stop DataNode"); + UTIL.getDFSCluster().stopDataNode(0); + assertEquals(2, UTIL.getDFSCluster().getDataNodes().size()); + + store.insert(new TestProcedure(1, -1), null); + for (long i = 2; store.isRunning(); ++i) { assertEquals(2, UTIL.getDFSCluster().getDataNodes().size()); - - store.insert(new TestProcedure(1, -1), null); - for (long i = 2; store.isRunning(); ++i) { - assertEquals(2, UTIL.getDFSCluster().getDataNodes().size()); - store.insert(new TestProcedure(i, -1), null); - Thread.sleep(100); - } - assertFalse(store.isRunning()); - fail("The store.insert() should throw an exeption"); - } finally { - tearDown(); + store.insert(new TestProcedure(i, -1), null); + Thread.sleep(100); } + assertFalse(store.isRunning()); + fail("The store.insert() should throw an exeption"); } @Test(timeout=60000) public void testWalAbortOnLowReplicationWithQueuedWriters() throws Exception { - initConfig(UTIL.getConfiguration()); setup(); - try { - assertEquals(3, UTIL.getDFSCluster().getDataNodes().size()); - store.registerListener(new ProcedureStore.ProcedureStoreListener() { - @Override - public void postSync() { - Threads.sleepWithoutInterrupt(2000); - } + assertEquals(3, UTIL.getDFSCluster().getDataNodes().size()); + store.registerListener(new ProcedureStore.ProcedureStoreListener() { + @Override + public void postSync() { + Threads.sleepWithoutInterrupt(2000); + } - @Override - public void abortProcess() {} - }); + @Override + public void abortProcess() {} + }); - final AtomicInteger reCount = new AtomicInteger(0); - Thread[] thread = new Thread[store.getNumThreads() * 2 + 1]; - for (int i = 0; i < thread.length; ++i) { - final long procId = i + 1; - thread[i] = new Thread() { - public void run() { - try { - LOG.debug("[S] INSERT " + procId); - store.insert(new TestProcedure(procId, -1), null); - LOG.debug("[E] INSERT " + procId); - } catch (RuntimeException e) { - reCount.incrementAndGet(); - LOG.debug("[F] INSERT " + procId + ": " + e.getMessage()); - } + final AtomicInteger reCount = new AtomicInteger(0); + Thread[] thread = new Thread[store.getNumThreads() * 2 + 1]; + for (int i = 0; i < thread.length; ++i) { + final long procId = i + 1; + thread[i] = new Thread() { + public void run() { + try { + LOG.debug("[S] INSERT " + procId); + store.insert(new TestProcedure(procId, -1), null); + LOG.debug("[E] INSERT " + procId); + } catch (RuntimeException e) { + reCount.incrementAndGet(); + LOG.debug("[F] INSERT " + procId + ": " + e.getMessage()); } - }; - thread[i].start(); - } - - Thread.sleep(1000); - LOG.info("Stop DataNode"); - UTIL.getDFSCluster().stopDataNode(0); - assertEquals(2, UTIL.getDFSCluster().getDataNodes().size()); - - for (int i = 0; i < thread.length; ++i) { - thread[i].join(); - } - - assertFalse(store.isRunning()); - assertTrue(reCount.toString(), reCount.get() >= store.getNumThreads() && - reCount.get() < thread.length); - } finally { - tearDown(); + } + }; + thread[i].start(); } + + Thread.sleep(1000); + LOG.info("Stop DataNode"); + UTIL.getDFSCluster().stopDataNode(0); + assertEquals(2, UTIL.getDFSCluster().getDataNodes().size()); + + for (int i = 0; i < thread.length; ++i) { + thread[i].join(); + } + + assertFalse(store.isRunning()); + assertTrue(reCount.toString(), reCount.get() >= store.getNumThreads() && + reCount.get() < thread.length); } @Test(timeout=60000) public void testWalRollOnLowReplication() throws Exception { - initConfig(UTIL.getConfiguration()); UTIL.getConfiguration().setInt("dfs.namenode.replication.min", 1); setup(); - try { - int dnCount = 0; - store.insert(new TestProcedure(1, -1), null); - UTIL.getDFSCluster().restartDataNode(dnCount); - for (long i = 2; i < 100; ++i) { - store.insert(new TestProcedure(i, -1), null); - waitForNumReplicas(3); - Thread.sleep(100); - if ((i % 30) == 0) { - LOG.info("Restart Data Node"); - UTIL.getDFSCluster().restartDataNode(++dnCount % 3); - } + int dnCount = 0; + store.insert(new TestProcedure(1, -1), null); + UTIL.getDFSCluster().restartDataNode(dnCount); + for (long i = 2; i < 100; ++i) { + store.insert(new TestProcedure(i, -1), null); + waitForNumReplicas(3); + Thread.sleep(100); + if ((i % 30) == 0) { + LOG.info("Restart Data Node"); + UTIL.getDFSCluster().restartDataNode(++dnCount % 3); } - assertTrue(store.isRunning()); - } finally { - tearDown(); } + assertTrue(store.isRunning()); } public void waitForNumReplicas(int numReplicas) throws Exception {