From 8a595dc187294bed731fe3592ae2f8024aae47bb Mon Sep 17 00:00:00 2001 From: Xu Cang Date: Fri, 17 Aug 2018 02:41:13 -0700 Subject: [PATCH] Improve isTableState() method to ensure caller gets correct info better handling IOException for getTableState --- .../apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java | 2 ++ .../org/apache/hadoop/hbase/master/TableStateManager.java | 11 ++++++++--- .../hadoop/hbase/master/assignment/AssignmentManager.java | 15 +++++++++++---- .../hadoop/hbase/master/assignment/RegionStateStore.java | 2 +- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java index b39d3a19a1..e2fc946920 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java @@ -409,6 +409,8 @@ public class RSGroupAdminServer implements RSGroupAdmin { // action is required. if (targetGroup != null) { for (TableName table: tables) { + // may throw IOException here when table state cannot be retrieved. + // It's good to throw exception than blindly ignoring this region if (master.getAssignmentManager().isTableDisabled(table)) { LOG.debug("Skipping move regions because the table" + table + " is disabled."); continue; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java index 6e3461873e..91c1abb21a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java @@ -110,7 +110,10 @@ public class TableStateManager { } else { return currentState; } - } finally { + } catch (IOException e) { + throw new TableNotFoundException(tableName); + } + finally { lock.writeLock().unlock(); } } @@ -142,14 +145,14 @@ public class TableStateManager { } } - public boolean isTableState(TableName tableName, TableState.State... states) { + public boolean isTableState(TableName tableName, TableState.State... states) throws IOException { try { TableState tableState = getTableState(tableName); return tableState.isInStates(states); } catch (IOException e) { LOG.error("Unable to get table " + tableName + " state", e); // XXX: is it safe to just return false here? - return false; + throw e; } } @@ -215,6 +218,8 @@ public class TableStateManager { throw new TableStateNotFoundException(tableName); } return currentState; + } catch (IOException e){ + throw new TableStateNotFoundException(tableName); } finally { lock.readLock().unlock(); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java index 70a96802f3..e0b106ee41 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java @@ -326,10 +326,16 @@ public class AssignmentManager implements ServerListener { } public boolean isTableEnabled(final TableName tableName) { - return getTableStateManager().isTableState(tableName, TableState.State.ENABLED); + try { + return getTableStateManager().isTableState(tableName, TableState.State.ENABLED); + } catch (IOException e) { + LOG.warn("Getting table state failed, table name: " + tableName); + return false; + } + } - public boolean isTableDisabled(final TableName tableName) { + public boolean isTableDisabled(final TableName tableName) throws IOException { return getTableStateManager().isTableState(tableName, TableState.State.DISABLED, TableState.State.DISABLING); } @@ -1250,7 +1256,8 @@ public class AssignmentManager implements ServerListener { regionStateStore.visitMeta(new RegionStateStore.RegionStateVisitor() { @Override public void visitRegionState(Result result, final RegionInfo regionInfo, final State state, - final ServerName regionLocation, final ServerName lastHost, final long openSeqNum) { + final ServerName regionLocation, final ServerName lastHost, final long openSeqNum) + throws IOException{ if (state == null && regionLocation == null && lastHost == null && openSeqNum == SequenceId.NO_SEQUENCE_ID) { // This is a row with nothing in it. @@ -1359,7 +1366,7 @@ public class AssignmentManager implements ServerListener { * @return Pair indicating the status of the alter command (pending/total) * @throws IOException */ - public Pair getReopenStatus(TableName tableName) { + public Pair getReopenStatus(TableName tableName) throws IOException { if (isTableDisabled(tableName)) return new Pair(0, 0); final List states = regionStates.getTableRegionStates(tableName); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java index aeef835dec..da87d23af6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java @@ -69,7 +69,7 @@ public class RegionStateStore { public interface RegionStateVisitor { void visitRegionState(Result result, RegionInfo regionInfo, State state, - ServerName regionLocation, ServerName lastHost, long openSeqNum); + ServerName regionLocation, ServerName lastHost, long openSeqNum) throws IOException; } public void visitMeta(final RegionStateVisitor visitor) throws IOException { -- 2.14.3 (Apple Git-98)