[
https://issues.apache.org/jira/browse/HBASE-8656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13678921#comment-13678921
]
cuijianwei commented on HBASE-8656:
-----------------------------------
[~andrew.purt...@gmail.com], sorry for replying late, I run unit tests for this
0.94 patch, it seems ok. Can we run this patch for 0.94?
> 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
> 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
