[
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.
