From fb654a9125d2ae6f2e43e4207749266cb694bffc Mon Sep 17 00:00:00 2001 From: shahrs87 Date: Tue, 1 Sep 2020 13:52:07 +0530 Subject: [PATCH] HBASE-24957 ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not Closes #2323 Signed-off-by: Viraj Jasani Signed-off-by: Bharath Vissapragada --- .../ZKTableStateClientSideReader.java | 24 +++++---- .../TestZKTableStateClientSideReader.java | 52 +++++++++++++++++++ .../hadoop/hbase/HBaseTestingUtility.java | 8 ++- .../hadoop/hbase/client/TestAdmin1.java | 21 +++++--- 4 files changed, 87 insertions(+), 18 deletions(-) create mode 100644 hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKTableStateClientSideReader.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTableStateClientSideReader.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTableStateClientSideReader.java index 03579fff45d..7c21b015426 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTableStateClientSideReader.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTableStateClientSideReader.java @@ -19,6 +19,7 @@ */ package org.apache.hadoop.hbase.zookeeper; +import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.exceptions.DeserializationException; @@ -55,7 +56,7 @@ public class ZKTableStateClientSideReader { */ public static boolean isDisabledTable(final ZooKeeperWatcher zkw, final TableName tableName) - throws KeeperException, InterruptedException { + throws KeeperException, InterruptedException, TableNotFoundException { ZooKeeperProtos.Table.State state = getTableState(zkw, tableName); return isTableState(ZooKeeperProtos.Table.State.DISABLED, state); } @@ -71,7 +72,7 @@ public class ZKTableStateClientSideReader { */ public static boolean isEnabledTable(final ZooKeeperWatcher zkw, final TableName tableName) - throws KeeperException, InterruptedException { + throws KeeperException, InterruptedException, TableNotFoundException { return getTableState(zkw, tableName) == ZooKeeperProtos.Table.State.ENABLED; } @@ -87,7 +88,7 @@ public class ZKTableStateClientSideReader { */ public static boolean isDisablingOrDisabledTable(final ZooKeeperWatcher zkw, final TableName tableName) - throws KeeperException, InterruptedException { + throws KeeperException, InterruptedException, TableNotFoundException { ZooKeeperProtos.Table.State state = getTableState(zkw, tableName); return isTableState(ZooKeeperProtos.Table.State.DISABLING, state) || isTableState(ZooKeeperProtos.Table.State.DISABLED, state); @@ -99,7 +100,7 @@ public class ZKTableStateClientSideReader { * @throws KeeperException */ public static Set getDisabledTables(ZooKeeperWatcher zkw) - throws KeeperException, InterruptedException { + throws KeeperException, InterruptedException, TableNotFoundException { Set disabledTables = new HashSet(); List children = ZKUtil.listChildrenNoWatch(zkw, zkw.tableZNode); @@ -118,7 +119,7 @@ public class ZKTableStateClientSideReader { * @throws KeeperException */ public static Set getDisabledOrDisablingTables(ZooKeeperWatcher zkw) - throws KeeperException, InterruptedException { + throws KeeperException, InterruptedException, TableNotFoundException { return getTablesInStates( zkw, @@ -134,7 +135,7 @@ public class ZKTableStateClientSideReader { * @throws InterruptedException */ public static Set getEnablingTables(ZooKeeperWatcher zkw) - throws KeeperException, InterruptedException { + throws KeeperException, InterruptedException, TableNotFoundException { return getTablesInStates(zkw, ZooKeeperProtos.Table.State.ENABLING); } @@ -149,7 +150,7 @@ public class ZKTableStateClientSideReader { private static Set getTablesInStates( ZooKeeperWatcher zkw, ZooKeeperProtos.Table.State... states) - throws KeeperException, InterruptedException { + throws KeeperException, InterruptedException, TableNotFoundException { Set tableNameSet = new HashSet(); List children = ZKUtil.listChildrenNoWatch(zkw, zkw.tableZNode); TableName tableName; @@ -175,15 +176,18 @@ public class ZKTableStateClientSideReader { /** * @param zkw ZooKeeperWatcher instance to use * @param tableName table we're checking - * @return Null or {@link ZooKeeperProtos.Table.State} found in znode. + * @return {@link ZooKeeperProtos.Table.State} found in znode. * @throws KeeperException + * @throws TableNotFoundException if tableName doesn't exist */ static ZooKeeperProtos.Table.State getTableState(final ZooKeeperWatcher zkw, final TableName tableName) - throws KeeperException, InterruptedException { + throws KeeperException, InterruptedException, TableNotFoundException { String znode = ZKUtil.joinZNode(zkw.tableZNode, tableName.getNameAsString()); byte [] data = ZKUtil.getData(zkw, znode); - if (data == null || data.length <= 0) return null; + if (data == null || data.length <= 0) { + throw new TableNotFoundException(tableName); + } try { ProtobufUtil.expectPBMagicPrefix(data); ZooKeeperProtos.Table.Builder builder = ZooKeeperProtos.Table.newBuilder(); diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKTableStateClientSideReader.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKTableStateClientSideReader.java new file mode 100644 index 00000000000..e82d3b0fc34 --- /dev/null +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKTableStateClientSideReader.java @@ -0,0 +1,52 @@ +/** + * Copyright The Apache Software Foundation + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.zookeeper; + +import static org.junit.Assert.fail; + +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.TableNotFoundException; +import org.apache.hadoop.hbase.testclassification.SmallTests; + +import org.apache.zookeeper.Watcher; +import org.apache.zookeeper.data.Stat; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.mockito.Mockito; + +@Category({SmallTests.class}) +public class TestZKTableStateClientSideReader { + + @Test + public void test() throws Exception { + ZooKeeperWatcher zkw = Mockito.mock(ZooKeeperWatcher.class); + RecoverableZooKeeper rzk = Mockito.mock(RecoverableZooKeeper.class); + Mockito.doReturn(rzk).when(zkw).getRecoverableZooKeeper(); + Mockito.doReturn(null).when(rzk).getData(Mockito.anyString(), + Mockito.any(Watcher.class), Mockito.any(Stat.class)); + TableName table = TableName.valueOf("table-not-exists"); + try { + ZKTableStateClientSideReader.getTableState(zkw, table); + fail("Shouldn't reach here"); + } catch(TableNotFoundException e) { + // Expected Table not found exception + } + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java index 3335b493aaa..ec1e32c2947 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java @@ -4222,7 +4222,13 @@ public class HBaseTestingUtility extends HBaseCommonTestingUtility { @Override public boolean evaluate() throws IOException { - return getHBaseAdmin().tableExists(tableName) && getHBaseAdmin().isTableEnabled(tableName); + try { + return getHBaseAdmin().tableExists(tableName) + && getHBaseAdmin().isTableEnabled(tableName); + } catch (TableNotFoundException tnfe) { + // Ignore TNFE + return false; + } } }; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java index 5885024fa16..c0b32b82a5b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java @@ -31,7 +31,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import org.apache.commons.logging.Log; @@ -42,8 +41,6 @@ import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.InvalidFamilyOperationException; -import org.apache.hadoop.hbase.testclassification.LargeTests; -import org.apache.hadoop.hbase.MasterNotRunningException; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; @@ -51,15 +48,13 @@ import org.apache.hadoop.hbase.TableNotDisabledException; import org.apache.hadoop.hbase.TableNotEnabledException; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.Waiter; -import org.apache.hadoop.hbase.executor.EventHandler; +import org.apache.hadoop.hbase.exceptions.MergeRegionException; import org.apache.hadoop.hbase.protobuf.generated.ZooKeeperProtos; +import org.apache.hadoop.hbase.testclassification.LargeTests; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.FSUtils; import org.apache.hadoop.hbase.zookeeper.ZKTableStateClientSideReader; -import org.apache.hadoop.hbase.ZooKeeperConnectionException; -import org.apache.hadoop.hbase.exceptions.MergeRegionException; import org.apache.hadoop.hbase.master.AssignmentManager; -import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.master.RegionState; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.protobuf.RequestConverter; @@ -1492,4 +1487,16 @@ public class TestAdmin1 { this.admin.deleteTable(tableName); } } + + @Test (timeout=30000) + public void testTableNotFoundException() throws Exception { + ZooKeeperWatcher zkw = TEST_UTIL.getZooKeeperWatcher(); + TableName table = TableName.valueOf("tableNotExists"); + try { + ZKTableStateClientSideReader.isDisabledTable(zkw, table); + fail("Shouldn't be here"); + } catch (TableNotFoundException e) { + // This is expected. + } + } }