[
https://issues.apache.org/jira/browse/HADOOP-2555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12558345#action_12558345
]
stack commented on HADOOP-2555:
-------------------------------
Pardon me. I wasn't thinking 1. or 2 (I do think HTable will not be well
tested -- or that we should be prepared for the 'unexpected'). You could stuff
the original 'unexpected' exception into a new instance of RuntimeException.
Then you wouldn't have to change the signature and it'd show as 'cause' in any
stack trace. IMO, this is preferable to returning null (null means row
doesn't exist), especially where HTable is embedded.
Anyways, just a suggestion. Patch is welcome if you think returning null
better.
> 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.
