Return-Path: Delivered-To: apmail-lucene-hadoop-commits-archive@locus.apache.org Received: (qmail 31080 invoked from network); 11 Jul 2007 14:23:24 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2) by minotaur.apache.org with SMTP; 11 Jul 2007 14:23:24 -0000 Received: (qmail 19334 invoked by uid 500); 11 Jul 2007 14:23:26 -0000 Delivered-To: apmail-lucene-hadoop-commits-archive@lucene.apache.org Received: (qmail 19307 invoked by uid 500); 11 Jul 2007 14:23:26 -0000 Mailing-List: contact hadoop-commits-help@lucene.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: hadoop-dev@lucene.apache.org Delivered-To: mailing list hadoop-commits@lucene.apache.org Received: (qmail 19298 invoked by uid 99); 11 Jul 2007 14:23:26 -0000 Received: from herse.apache.org (HELO herse.apache.org) (140.211.11.133) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 11 Jul 2007 07:23:26 -0700 X-ASF-Spam-Status: No, hits=-99.5 required=10.0 tests=ALL_TRUSTED,NO_REAL_NAME X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO eris.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 11 Jul 2007 07:23:22 -0700 Received: by eris.apache.org (Postfix, from userid 65534) id EC0DF1A981F; Wed, 11 Jul 2007 07:23:01 -0700 (PDT) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r555282 - in /lucene/hadoop/trunk/src/contrib/hbase: CHANGES.txt src/java/org/apache/hadoop/hbase/HClient.java src/java/org/apache/hadoop/hbase/HMaster.java src/java/org/apache/hadoop/hbase/RemoteExceptionHandler.java Date: Wed, 11 Jul 2007 14:23:01 -0000 To: hadoop-commits@lucene.apache.org From: jimk@apache.org X-Mailer: svnmailer-1.1.0 Message-Id: <20070711142301.EC0DF1A981F@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: jimk Date: Wed Jul 11 07:23:00 2007 New Revision: 555282 URL: http://svn.apache.org/viewvc?view=rev&rev=555282 Log: Exception handling in HBase is broken over client server connections Added: lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/RemoteExceptionHandler.java Modified: lucene/hadoop/trunk/src/contrib/hbase/CHANGES.txt lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HClient.java lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HMaster.java Modified: lucene/hadoop/trunk/src/contrib/hbase/CHANGES.txt URL: http://svn.apache.org/viewvc/lucene/hadoop/trunk/src/contrib/hbase/CHANGES.txt?view=diff&rev=555282&r1=555281&r2=555282 ============================================================================== --- lucene/hadoop/trunk/src/contrib/hbase/CHANGES.txt (original) +++ lucene/hadoop/trunk/src/contrib/hbase/CHANGES.txt Wed Jul 11 07:23:00 2007 @@ -55,5 +55,6 @@ 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 Modified: lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HClient.java URL: http://svn.apache.org/viewvc/lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HClient.java?view=diff&rev=555282&r1=555281&r2=555282 ============================================================================== --- lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HClient.java (original) +++ lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HClient.java Wed Jul 11 07:23:00 2007 @@ -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 @@ } 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 @@ checkMaster(); try { this.master.createTable(desc); - } catch (RemoteException e) { - handleRemoteException(e); + } catch (Exception e) { + RemoteExceptionHandler.handleRemoteException(e); } } @@ -302,8 +279,8 @@ 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 @@ 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 @@ try { this.master.addColumn(tableName, column); - } catch(RemoteException e) { - handleRemoteException(e); + } catch (Exception e) { + RemoteExceptionHandler.handleRemoteException(e); } } @@ -386,8 +368,8 @@ try { this.master.deleteColumn(tableName, columnName); - } catch(RemoteException e) { - handleRemoteException(e); + } catch(Exception e) { + RemoteExceptionHandler.handleRemoteException(e); } } @@ -405,8 +387,8 @@ 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 @@ break; } + } catch (Exception e) { + if(tries == numRetries - 1) { // no more retries + RemoteExceptionHandler.handleRemoteException(e); + } + } finally { if(scannerId != -1L) { try { @@ -488,8 +475,8 @@ 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 @@ break; } + } catch(Exception e) { + if(tries == numRetries - 1) { // no more retries + RemoteExceptionHandler.handleRemoteException(e); + } + } finally { if(scannerId != -1L) { try { @@ -561,7 +553,11 @@ */ public synchronized void shutdown() throws IOException { checkMaster(); - this.master.shutdown(); + try { + this.master.shutdown(); + } catch(Exception e) { + RemoteExceptionHandler.handleRemoteException(e); + } } /* @@ -741,10 +737,10 @@ 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 @@ TreeMap servers = new TreeMap(); for(int tries = 0; servers.size() == 0 && tries < this.numRetries; tries++) { - + long scannerId = -1L; try { scannerId = @@ -822,13 +818,13 @@ 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 @@ 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 @@ } try { Thread.sleep(this.pause); - } catch (InterruptedException e) { + } catch (InterruptedException ie) { // continue } if (LOG.isDebugEnabled()) { @@ -907,8 +908,8 @@ * @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 @@ 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 @@ } } } + } catch (Exception ex) { + RemoteExceptionHandler.handleRemoteException(ex); + } finally { if(scannerId != -1L) { server.close(scannerId); @@ -1046,19 +1057,26 @@ * @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 @@ * @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 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 @@ * @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 @@ 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 @@ } this.currentServer = null; this.currentRegion = null; - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } @@ -1360,7 +1397,7 @@ 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 @@ } this.currentServer = null; this.currentRegion = null; - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } @@ -1381,10 +1418,10 @@ 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 @@ 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 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 @@ } this.currentServer = null; this.currentRegion = null; - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } @@ -1521,19 +1559,19 @@ 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; } Modified: lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HMaster.java URL: http://svn.apache.org/viewvc/lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HMaster.java?view=diff&rev=555282&r1=555281&r2=555282 ============================================================================== --- lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HMaster.java (original) +++ lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/HMaster.java Wed Jul 11 07:23:00 2007 @@ -1205,11 +1205,13 @@ 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 @@ 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 @@ } break; - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } } @@ -1536,9 +1538,9 @@ break; - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { - throw e; + RemoteExceptionHandler.handleRemoteException(e); } continue; } @@ -1647,9 +1649,9 @@ break; - } catch(NotServingRegionException e) { + } catch(Exception e) { if(tries == numRetries - 1) { - throw e; + RemoteExceptionHandler.handleRemoteException(e); } } pendingRegions.remove(regionName); @@ -1752,9 +1754,9 @@ 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 @@ } // 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 @@ 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 @@ 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) { Added: lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/RemoteExceptionHandler.java URL: http://svn.apache.org/viewvc/lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/RemoteExceptionHandler.java?view=auto&rev=555282 ============================================================================== --- lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/RemoteExceptionHandler.java (added) +++ lucene/hadoop/trunk/src/contrib/hbase/src/java/org/apache/hadoop/hbase/RemoteExceptionHandler.java Wed Jul 11 07:23:00 2007 @@ -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; + } + } +}