hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Hofhansl (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-8656) Rpc call may not be notified in SecureClient
Date Fri, 21 Jun 2013 21:14:20 GMT

    [ https://issues.apache.org/jira/browse/HBASE-8656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13690724#comment-13690724
] 

Lars Hofhansl commented on HBASE-8656:
--------------------------------------

+1 on patch. Makes sense and brings it (more) in line with HBaseClient.
                
> Rpc call may not be notified in SecureClient
> --------------------------------------------
>
>                 Key: HBASE-8656
>                 URL: https://issues.apache.org/jira/browse/HBASE-8656
>             Project: HBase
>          Issue Type: Bug
>          Components: Client, IPC/RPC, security
>    Affects Versions: 0.94.7
>            Reporter: cuijianwei
>            Assignee: cuijianwei
>            Priority: Critical
>             Fix For: 0.94.9
>
>         Attachments: HBASE-8656-0.94-v1.txt
>
>
> In SecureClient.java, rpc responses will be processed by receiveResponse() which looks
like:
> {code}
> try {
>         int id = in.readInt();                    // try to read an id
>         if (LOG.isDebugEnabled())
>           LOG.debug(getName() + " got value #" + id);
>         Call call = calls.remove(id);
>         int state = in.readInt();     // read call status
>         if (LOG.isDebugEnabled()) {
>           LOG.debug("call #"+id+" state is " + state);
>         }
>         if (state == Status.SUCCESS.state) {
>           Writable value = ReflectionUtils.newInstance(valueClass, conf);
>           value.readFields(in);                 // read value
>           if (LOG.isDebugEnabled()) {
>             LOG.debug("call #"+id+", response is:\n"+value.toString());
>           }
>           // it's possible that this call may have been cleaned up due to a RPC
>           // timeout, so check if it still exists before setting the value.
>           if (call != null) {
>             call.setValue(value);
>           }
>         } else if (state == Status.ERROR.state) {
>           if (call != null) {
>             call.setException(new RemoteException(WritableUtils.readString(in), WritableUtils
>                 .readString(in)));
>           }
>         } else if (state == Status.FATAL.state) {
>           // Close the connection
>           markClosed(new RemoteException(WritableUtils.readString(in),
>                                          WritableUtils.readString(in)));
>         }
>       } catch (IOException e) {
>         if (e instanceof SocketTimeoutException && remoteId.rpcTimeout > 0)
{
>           // Clean up open calls but don't treat this as a fatal condition,
>           // since we expect certain responses to not make it by the specified
>           // {@link ConnectionId#rpcTimeout}.
>           closeException = e;
>         } else {
>           // Since the server did not respond within the default ping interval
>           // time, treat this as a fatal condition and close this connection
>           markClosed(e);
>         }
>       } finally {
>         if (remoteId.rpcTimeout > 0) {
>           cleanupCalls(remoteId.rpcTimeout);
>         }
>       }
>     }
> {code}
> In above code, in the try block, the call will be firstly removed from call map by:
> {code}
>         Call call = calls.remove(id);
> {code}
> There may be two cases leading the call couldn't be notified and the invoking thread
will wait forever. 
> Firstly, if the returned status is Status.FATAL.state by:
> {code}
>         int state = in.readInt();     // read call status
> {code}
> The code will come into:
> {code}
> } else if (state == Status.FATAL.state) {
>           // Close the connection
>           markClosed(new RemoteException(WritableUtils.readString(in),
>                                          WritableUtils.readString(in)));
>         }
> {code}
> Here, the SecureConnection is marked as closed and all rpc calls in call map of this
connection will be notified to receive an exception. However, the current rpc call has been
removed from the call map, it won't be notified.
> Secondly, after the call has been removed by:
> {code}
>         Call call = calls.remove(id);
> {code}
> If we encounter any exception before the 'try' block finished, the code will come into
'catch' and 'finally' block, neither 'catch' block nor 'finally' block will notify the rpc
call because it has been removed from call map.
> Compared with receiveResponse() in HBaseClient.java, it may be better to get the rpc
call from call map and remove it at the end of the 'try' block.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message