hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Peter Dolan (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HADOOP-2555) Refactor the HTable#get and HTable#getRow methods to avoid repetition of retry-on-failure logic
Date Sun, 13 Jan 2008 03:24:33 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-2555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12558339#action_12558339
] 

Peter Dolan commented on HADOOP-2555:
-------------------------------------

Bryan: Excellent suggestion, I was initially thinking along those lines but wasn't sure how
general or specific to make the modification -- I suppose all of the get... methods will need
to have their row's region location and server reloaded before the call is retried, so I've
integrated your suggestion.

Stack: I guess this is the eternal balancing act.  My feelings were that non-IOExceptions
thrown by the the Callable#call method are definitely HTable bugs and should really just be
fully tested.  It seems that there are two alternatives to swallowing and logging the exception,
neither of which are satisfying to me:

1) Wrap the UnexpectedCalalbleException in an IOException, or have UnexpectedCallableException
descend from IOException.  This isn't satisfying from a semantic standpoint, as UnexpectedCallableExceptions
are produced precisely by all exceptions which are _not_ IOExceptions.

2) Rethrow the UnexpectedCallableExceptions.  This is unsatisfying because the get methods
are public, and having get(...) throw both IOException and UnexpectedCallableException seems
to introduce too many specifics of HTable's internal details to the public API.

Because both of those options are really pretty dissatisfying to me, I think that the HTable
class should swallow and log those exceptions and be very well tested. 

> Refactor the HTable#get and HTable#getRow methods to avoid repetition of retry-on-failure
logic
> -----------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-2555
>                 URL: https://issues.apache.org/jira/browse/HADOOP-2555
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: contrib/hbase
>            Reporter: Peter Dolan
>            Priority: Minor
>         Attachments: hadoop-2555.patch
>
>
> The following code is repeated in every one of HTable#get and HTable#getRow methods:
> {code:title=HTable.java|borderStyle=solid}
>     MapWritable value = null;
>     for (int tries = 0; tries < numRetries; tries++) {
>       HRegionLocation r = getRegionLocation(row);
>       HRegionInterface server =
>         connection.getHRegionConnection(r.getServerAddress());
>       
>       try {
>         value = server.getRow(r.getRegionInfo().getRegionName(), row, ts);  // This is
the only line of code that changes significantly between methods
>         break;
>         
>       } catch (IOException e) {
>         if (e instanceof RemoteException) {
>           e = RemoteExceptionHandler.decodeRemoteException((RemoteException) e);
>         }
>         if (tries == numRetries - 1) {
>           // No more tries
>           throw e;
>         }
>         if (LOG.isDebugEnabled()) {
>           LOG.debug("reloading table servers because: " + e.getMessage());
>         }
>         tableServers = connection.reloadTableServers(tableName);
>       }
>       try {
>         Thread.sleep(this.pause);
>         
>       } catch (InterruptedException x) {
>         // continue
>       }
>     }
> {code}
> This should be factored out into a protected method that handles retry-on-failure logic
to facilitate more robust testing and the development of new API methods.
> Proposed modification:
> // Execute the provided Callable against the server
> protected <T> callServerWithRetries(Callable<T> callable) throws RemoteException;
> The above code could then be reduced to:
> {code:title=HTable.java|borderStyle=solid}
>     MapWritable value = null;
>     final connection;
>     try {
>       value = callServerWithRetries(new Callable<MapWritable>() {
>             HRegionLocation r = getRegionLocation(row);
>             HRegionInterface server =
>                 connection.getHRegionConnection(r.getServerAddress());
>             server.getRow(r.getRegionInfo().getRegionName(), row, ts);
>           });
>     } catch (RemoteException e) {
>       // handle unrecoverable remote exceptions
>     }
> {code}
> This would greatly ease the development of new API methods by reducing the amount of
code needed to implement a new method and reducing the amount of logic that needs to be tested
per method.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message