[GitHub] [hbase] shahrs87 commented on a change in pull request #2323: [HBASE-24957] ZKTableStateClientSideReader#isDisabledTable doesn't check if table exists or not.

[GitBox <gi...@apache.org>]

shahrs87 commented on a change in pull request #2323:
URL: https://github.com/apache/hbase/pull/2323#discussion_r479813867



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKTableStateClientSideReader.java
##########
@@ -175,15 +176,18 @@ static boolean isTableState(final ZooKeeperProtos.Table.State expectedState,
   /**
    * @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);

Review comment:
       > all callers of all these methods have handled change accordingly? 
   
   All the callers throws IOException and TNFE is a sub class of IOException. So no need to handle it explicitly.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org