From f613907a98a732364c5a2a5f1f4ece7bc99f555e Mon Sep 17 00:00:00 2001 From: Jim Kellerman Date: Wed, 11 Jul 2007 14:23:00 +0000 Subject: [PATCH] Exception handling in HBase is broken over client server connections git-svn-id: https://svn.apache.org/repos/asf/lucene/hadoop/trunk/src/contrib/hbase@555282 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 1 + src/java/org/apache/hadoop/hbase/HClient.java | 244 ++++++++++-------- src/java/org/apache/hadoop/hbase/HMaster.java | 47 ++-- .../hadoop/hbase/RemoteExceptionHandler.java | 91 +++++++ 4 files changed, 256 insertions(+), 127 deletions(-) create mode 100644 src/java/org/apache/hadoop/hbase/RemoteExceptionHandler.java diff --git a/CHANGES.txt b/CHANGES.txt index 0e35d4377f1..fc4c5ee1bec 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -55,5 +55,6 @@ Trunk (unreleased changes) HADOOP-1466 Clean up visibility and javadoc issues in HBase. 33. HADOOP-1538 Provide capability for client specified time stamps in HBase HADOOP-1466 Clean up visibility and javadoc issues in HBase. + 34. HADOOP-1589 Exception handling in HBase is broken over client server connections diff --git a/src/java/org/apache/hadoop/hbase/HClient.java b/src/java/org/apache/hadoop/hbase/HClient.java index 64a69bb11cf..48384d5fee4 100644 --- a/src/java/org/apache/hadoop/hbase/HClient.java +++ b/src/java/org/apache/hadoop/hbase/HClient.java @@ -15,6 +15,8 @@ */ package org.apache.hadoop.hbase; +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; @@ -153,32 +155,7 @@ public class HClient implements HConstants { } return result; } - - protected void handleRemoteException(RemoteException e) throws IOException { - String msg = e.getMessage(); - if(e.getClassName().equals("org.apache.hadoop.hbase.InvalidColumnNameException")) { - throw new InvalidColumnNameException(msg); - - } else if(e.getClassName().equals("org.apache.hadoop.hbase.LockException")) { - throw new LockException(msg); - - } else if(e.getClassName().equals("org.apache.hadoop.hbase.MasterNotRunningException")) { - throw new MasterNotRunningException(msg); - - } else if(e.getClassName().equals("org.apache.hadoop.hbase.NoServerForRegionException")) { - throw new NoServerForRegionException(msg); - - } else if(e.getClassName().equals("org.apache.hadoop.hbase.NotServingRegionException")) { - throw new NotServingRegionException(msg); - - } else if(e.getClassName().equals("org.apache.hadoop.hbase.TableNotDisabledException")) { - throw new TableNotDisabledException(msg); - - } else { - throw e; - } - } - + /* Find the address of the master and connect to it */ protected void checkMaster() throws MasterNotRunningException { @@ -284,8 +261,8 @@ public class HClient implements HConstants { checkMaster(); try { this.master.createTable(desc); - } catch (RemoteException e) { - handleRemoteException(e); + } catch (Exception e) { + RemoteExceptionHandler.handleRemoteException(e); } } @@ -302,8 +279,8 @@ public class HClient implements HConstants { try { this.master.deleteTable(tableName); - } catch(RemoteException e) { - handleRemoteException(e); + } catch(Exception e) { + RemoteExceptionHandler.handleRemoteException(e); } // Wait until first region is deleted @@ -333,13 +310,18 @@ public class HClient implements HConstants { if(!found) { break; } - + + } catch (Exception ex) { + if(tries == numRetries - 1) { // no more tries left + RemoteExceptionHandler.handleRemoteException(ex); + } + } finally { if(scannerId != -1L) { try { server.close(scannerId); - } catch(Exception e) { - LOG.warn(e); + } catch(Exception ex) { + LOG.warn(ex); } } } @@ -367,8 +349,8 @@ public class HClient implements HConstants { try { this.master.addColumn(tableName, column); - } catch(RemoteException e) { - handleRemoteException(e); + } catch (Exception e) { + RemoteExceptionHandler.handleRemoteException(e); } } @@ -386,8 +368,8 @@ public class HClient implements HConstants { try { this.master.deleteColumn(tableName, columnName); - } catch(RemoteException e) { - handleRemoteException(e); + } catch(Exception e) { + RemoteExceptionHandler.handleRemoteException(e); } } @@ -405,8 +387,8 @@ public class HClient implements HConstants { try { this.master.enableTable(tableName); - } catch(RemoteException e) { - handleRemoteException(e); + } catch(Exception e) { + RemoteExceptionHandler.handleRemoteException(e); } // Wait until first region is enabled @@ -447,6 +429,11 @@ public class HClient implements HConstants { break; } + } catch (Exception e) { + if(tries == numRetries - 1) { // no more retries + RemoteExceptionHandler.handleRemoteException(e); + } + } finally { if(scannerId != -1L) { try { @@ -488,8 +475,8 @@ public class HClient implements HConstants { try { this.master.disableTable(tableName); - } catch(RemoteException e) { - handleRemoteException(e); + } catch(Exception e) { + RemoteExceptionHandler.handleRemoteException(e); } // Wait until first region is disabled @@ -530,6 +517,11 @@ public class HClient implements HConstants { break; } + } catch(Exception e) { + if(tries == numRetries - 1) { // no more retries + RemoteExceptionHandler.handleRemoteException(e); + } + } finally { if(scannerId != -1L) { try { @@ -561,7 +553,11 @@ public class HClient implements HConstants { */ public synchronized void shutdown() throws IOException { checkMaster(); - this.master.shutdown(); + try { + this.master.shutdown(); + } catch(Exception e) { + RemoteExceptionHandler.handleRemoteException(e); + } } /* @@ -741,10 +737,10 @@ public class HClient implements HConstants { try { rootRegion.getRegionInfo(HGlobals.rootRegionInfo.regionName); break; - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { // Don't bother sleeping. We've run out of retries. - break; + RemoteExceptionHandler.handleRemoteException(e); } // Sleep and retry finding root region. @@ -806,7 +802,7 @@ public class HClient implements HConstants { TreeMap servers = new TreeMap(); for(int tries = 0; servers.size() == 0 && tries < this.numRetries; tries++) { - + long scannerId = -1L; try { scannerId = @@ -822,13 +818,13 @@ public class HClient implements HConstants { if(servers.size() == 0) { // If we didn't find any servers then the table does not exist throw new RegionNotFoundException("table '" + tableName + - "' does not exist in " + t); + "' does not exist in " + t); } // We found at least one server for the table and now we're done. if (LOG.isDebugEnabled()) { LOG.debug("Found " + servers.size() + " server(s) for " + - "location: " + t + " for tablename " + tableName); + "location: " + t + " for tablename " + tableName); } break; } @@ -868,19 +864,24 @@ public class HClient implements HConstants { servers.put(regionInfo.startKey, new RegionLocation(regionInfo, new HServerAddress(serverAddress))); } + } catch (Exception e) { + if(tries == numRetries - 1) { // no retries left + RemoteExceptionHandler.handleRemoteException(e); + } + } finally { if(scannerId != -1L) { try { server.close(scannerId); - } catch(Exception e) { - LOG.warn(e); + } catch(Exception ex) { + LOG.warn(ex); } } } - + if(servers.size() == 0 && tries == this.numRetries - 1) { throw new NoServerForRegionException("failed to find server for " - + tableName + " after " + this.numRetries + " retries"); + + tableName + " after " + this.numRetries + " retries"); } if (servers.size() <= 0) { @@ -891,7 +892,7 @@ public class HClient implements HConstants { } try { Thread.sleep(this.pause); - } catch (InterruptedException e) { + } catch (InterruptedException ie) { // continue } if (LOG.isDebugEnabled()) { @@ -907,8 +908,8 @@ public class HClient implements HConstants { * @param regionServer - the server to connect to * @throws IOException */ - protected synchronized HRegionInterface getHRegionConnection( - HServerAddress regionServer) throws IOException{ + protected synchronized HRegionInterface getHRegionConnection ( + HServerAddress regionServer) throws IOException { getRegionServerInterface(); @@ -918,22 +919,29 @@ public class HClient implements HConstants { if (server == null) { // Get a connection long versionId = 0; try { - versionId = serverInterfaceClass.getDeclaredField("versionID").getLong(server); + versionId = + serverInterfaceClass.getDeclaredField("versionID").getLong(server); + } catch (IllegalAccessException e) { // Should never happen unless visibility of versionID changes throw new UnsupportedOperationException( - "Unable to open a connection to a " + serverInterfaceClass.getName() + " server.", e); + "Unable to open a connection to a " + serverInterfaceClass.getName() + + " server.", e); + } catch (NoSuchFieldException e) { // Should never happen unless versionID field name changes in HRegionInterface throw new UnsupportedOperationException( - "Unable to open a connection to a " + serverInterfaceClass.getName() + " server.", e); + "Unable to open a connection to a " + serverInterfaceClass.getName() + + " server.", e); } - server = (HRegionInterface) RPC.waitForProxy( - serverInterfaceClass, - versionId, - regionServer.getInetSocketAddress(), - this.conf); + try { + server = (HRegionInterface) RPC.waitForProxy(serverInterfaceClass, + versionId, regionServer.getInetSocketAddress(), this.conf); + + } catch (Exception e) { + RemoteExceptionHandler.handleRemoteException(e); + } this.servers.put(regionServer.toString(), server); } @@ -988,6 +996,9 @@ public class HClient implements HConstants { } } } + } catch (Exception ex) { + RemoteExceptionHandler.handleRemoteException(ex); + } finally { if(scannerId != -1L) { server.close(scannerId); @@ -1046,19 +1057,26 @@ public class HClient implements HConstants { * @throws IOException */ public byte[] get(Text row, Text column) throws IOException { - RegionLocation info = null; byte [] value = null; - for(int tries = 0; tries < numRetries && info == null; tries++) { - info = getRegionLocation(row); + for(int tries = 0; tries < numRetries; tries++) { + RegionLocation info = getRegionLocation(row); + HRegionInterface server = getHRegionConnection(info.serverAddress); try { - value = getHRegionConnection(info.serverAddress). - get(info.regionInfo.regionName, row, column); - } catch (NotServingRegionException e) { + value = server.get(info.regionInfo.regionName, row, column); + break; + + } catch (Exception e) { if (tries == numRetries - 1) { - throw e; + RemoteExceptionHandler.handleRemoteException(e); } findRegion(info); } + try { + Thread.sleep(this.pause); + + } catch (InterruptedException x) { + // continue + } } return value; } @@ -1073,20 +1091,28 @@ public class HClient implements HConstants { * @throws IOException */ public byte[][] get(Text row, Text column, int numVersions) throws IOException { - RegionLocation info = null; byte [][] values = null; - for(int tries = 0; tries < numRetries && info == null; tries++) { - info = getRegionLocation(row); + for(int tries = 0; tries < numRetries; tries++) { + RegionLocation info = getRegionLocation(row); + HRegionInterface server = getHRegionConnection(info.serverAddress); + try { - values = getHRegionConnection(info.serverAddress).get( - info.regionInfo.regionName, row, column, numVersions); - } catch(NotServingRegionException e) { + values = server.get(info.regionInfo.regionName, row, column, numVersions); + break; + + } catch(Exception e) { if(tries == numRetries - 1) { // No more tries - throw e; + RemoteExceptionHandler.handleRemoteException(e); } findRegion(info); } + try { + Thread.sleep(this.pause); + + } catch (InterruptedException x) { + // continue + } } if(values != null) { @@ -1112,21 +1138,27 @@ public class HClient implements HConstants { */ public byte[][] get(Text row, Text column, long timestamp, int numVersions) throws IOException { - RegionLocation info = null; byte [][] values = null; - for(int tries = 0; tries < numRetries && info == null; tries++) { - info = getRegionLocation(row); + for(int tries = 0; tries < numRetries; tries++) { + RegionLocation info = getRegionLocation(row); + HRegionInterface server = getHRegionConnection(info.serverAddress); try { - values = getHRegionConnection(info.serverAddress). - get(info.regionInfo.regionName, row, column, timestamp, numVersions); + values = server.get(info.regionInfo.regionName, row, column, timestamp, numVersions); + break; - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { // No more tries - throw e; + RemoteExceptionHandler.handleRemoteException(e); } findRegion(info); } + try { + Thread.sleep(this.pause); + + } catch (InterruptedException x) { + // continue + } } if(values != null) { @@ -1147,23 +1179,28 @@ public class HClient implements HConstants { * @throws IOException */ public SortedMap getRow(Text row) throws IOException { - RegionLocation info = null; KeyedData[] value = null; - - for(int tries = 0; tries < numRetries && info == null; tries++) { - info = getRegionLocation(row); + for(int tries = 0; tries < numRetries; tries++) { + RegionLocation info = getRegionLocation(row); + HRegionInterface server = getHRegionConnection(info.serverAddress); try { - value = getHRegionConnection(info.serverAddress).getRow( - info.regionInfo.regionName, row); + value = server.getRow(info.regionInfo.regionName, row); + break; } catch(NotServingRegionException e) { if(tries == numRetries - 1) { // No more tries - throw e; + RemoteExceptionHandler.handleRemoteException(e); } findRegion(info); } + try { + Thread.sleep(this.pause); + + } catch (InterruptedException x) { + // continue + } } TreeMap results = new TreeMap(); if(value != null && value.length != 0) { @@ -1337,7 +1374,7 @@ public class HClient implements HConstants { try { this.currentServer.put(this.currentRegion, this.clientid, lockid, column, val); - } catch(IOException e) { + } catch(Exception e) { try { this.currentServer.abort(this.currentRegion, this.clientid, lockid); } catch(IOException e2) { @@ -1345,7 +1382,7 @@ public class HClient implements HConstants { } this.currentServer = null; this.currentRegion = null; - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } @@ -1360,7 +1397,7 @@ public class HClient implements HConstants { try { this.currentServer.delete(this.currentRegion, this.clientid, lockid, column); - } catch(IOException e) { + } catch(Exception e) { try { this.currentServer.abort(this.currentRegion, this.clientid, lockid); } catch(IOException e2) { @@ -1368,7 +1405,7 @@ public class HClient implements HConstants { } this.currentServer = null; this.currentRegion = null; - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } @@ -1381,10 +1418,10 @@ public class HClient implements HConstants { public void abort(long lockid) throws IOException { try { this.currentServer.abort(this.currentRegion, this.clientid, lockid); - } catch(IOException e) { + } catch(Exception e) { this.currentServer = null; this.currentRegion = null; - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } @@ -1410,9 +1447,10 @@ public class HClient implements HConstants { this.currentServer.commit(this.currentRegion, this.clientid, lockid, timestamp); - } finally { + } catch (Exception e) { this.currentServer = null; this.currentRegion = null; + RemoteExceptionHandler.handleRemoteException(e); } } @@ -1425,7 +1463,7 @@ public class HClient implements HConstants { public void renewLease(long lockid) throws IOException { try { this.currentServer.renewLease(lockid, this.clientid); - } catch(IOException e) { + } catch(Exception e) { try { this.currentServer.abort(this.currentRegion, this.clientid, lockid); } catch(IOException e2) { @@ -1433,7 +1471,7 @@ public class HClient implements HConstants { } this.currentServer = null; this.currentRegion = null; - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } @@ -1521,19 +1559,19 @@ public class HClient implements HConstants { break; - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { // No more tries - throw e; + RemoteExceptionHandler.handleRemoteException(e); } findRegion(info); loadRegions(); } } - } catch(IOException e) { + } catch(Exception e) { close(); - throw e; + RemoteExceptionHandler.handleRemoteException(e); } return true; } diff --git a/src/java/org/apache/hadoop/hbase/HMaster.java b/src/java/org/apache/hadoop/hbase/HMaster.java index d9378fb00b2..dcb0eaa4bf4 100644 --- a/src/java/org/apache/hadoop/hbase/HMaster.java +++ b/src/java/org/apache/hadoop/hbase/HMaster.java @@ -1205,11 +1205,13 @@ public class HMaster implements HConstants, HMasterInterface, try { values = server.next(scannerId); - } catch(NotServingRegionException e) { - throw e; - - } catch(IOException e) { - LOG.error(e); + } catch(Exception e) { + try { + RemoteExceptionHandler.handleRemoteException(e); + + } catch(Exception ex) { + LOG.error(ex); + } break; } @@ -1405,9 +1407,9 @@ public class HMaster implements HConstants, HMasterInterface, scanMetaRegion(server, scannerId, HGlobals.rootRegionInfo.regionName); break; - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } } @@ -1437,9 +1439,9 @@ public class HMaster implements HConstants, HMasterInterface, } break; - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } } @@ -1536,9 +1538,9 @@ public class HMaster implements HConstants, HMasterInterface, break; - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { - throw e; + RemoteExceptionHandler.handleRemoteException(e); } continue; } @@ -1647,9 +1649,9 @@ public class HMaster implements HConstants, HMasterInterface, break; - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } pendingRegions.remove(regionName); @@ -1752,9 +1754,9 @@ public class HMaster implements HConstants, HMasterInterface, assignAttempts.put(regionName, Long.valueOf(0L)); break; - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } } @@ -1941,9 +1943,9 @@ public class HMaster implements HConstants, HMasterInterface, } // for(MetaRegion m:) } // synchronized(metaScannerLock) - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { - throw e; + RemoteExceptionHandler.handleRemoteException(e); } continue; } @@ -2028,12 +2030,10 @@ public class HMaster implements HConstants, HMasterInterface, LOG.debug("updated columns in row: " + i.regionName); } - } catch(NotServingRegionException e) { - throw e; - - } catch(IOException e) { + } catch(Exception e) { LOG.error("column update failed in row: " + i.regionName); LOG.error(e); + RemoteExceptionHandler.handleRemoteException(e); } finally { try { @@ -2182,11 +2182,10 @@ public class HMaster implements HConstants, HMasterInterface, if(LOG.isDebugEnabled()) { LOG.debug("updated columns in row: " + i.regionName); } - } catch(NotServingRegionException e) { - throw e; - } catch(IOException e) { + } catch(Exception e) { LOG.error("column update failed in row: " + i.regionName); LOG.error(e); + RemoteExceptionHandler.handleRemoteException(e); } finally { if(lockid != -1L) { diff --git a/src/java/org/apache/hadoop/hbase/RemoteExceptionHandler.java b/src/java/org/apache/hadoop/hbase/RemoteExceptionHandler.java new file mode 100644 index 00000000000..5b23ff8c6ed --- /dev/null +++ b/src/java/org/apache/hadoop/hbase/RemoteExceptionHandler.java @@ -0,0 +1,91 @@ +/** + * Copyright 2007 The Apache Software Foundation + * + * Licensed 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; + +import java.io.IOException; +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; + +import org.apache.hadoop.ipc.RemoteException; + +/** + * An immutable class which contains a static method for handling + * org.apache.hadoop.ipc.RemoteException exceptions. + */ +public class RemoteExceptionHandler { + private RemoteExceptionHandler(){} // not instantiable + + /** + * Converts org.apache.hadoop.ipc.RemoteException into original exception, + * if possible. + * + * @param e original exception + * @throws IOException + */ + @SuppressWarnings("unchecked") + public static void handleRemoteException(final Exception e) throws IOException { + Exception ex = e; + if (e instanceof RemoteException) { + RemoteException r = (RemoteException) e; + + Class c = null; + try { + c = Class.forName(r.getClassName()); + + } catch (ClassNotFoundException x) { + throw r; + } + + Constructor ctor = null; + try { + Class[] parameterTypes = { String.class }; + ctor = c.getConstructor(parameterTypes); + + } catch (NoSuchMethodException x) { + throw r; + } + + try { + Object[] arguments = { r.getMessage() }; + + ex = (Exception) ctor.newInstance(arguments); + + } catch (IllegalAccessException x) { + throw r; + + } catch (InvocationTargetException x) { + throw r; + + } catch (InstantiationException x) { + throw r; + } + } + + if (ex instanceof IOException) { + IOException io = (IOException) ex; + throw io; + + } else if (ex instanceof RuntimeException) { + RuntimeException re = (RuntimeException) ex; + throw re; + + } else { + AssertionError a = new AssertionError("unexpected exception"); + a.initCause(ex); + throw a; + } + } +}