hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gary Helmling (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-3939) Some crossports of Hadoop IPC fixes
Date Thu, 03 Nov 2011 23:11:32 GMT

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

Gary Helmling commented on HBASE-3939:

Would be easier to review if the latest patch was in review board.

Here are comments from what I see:

In {{HBaseServer}}:
# In {{setupResponse()}} we take a {{Status}} instance (which is good), but it doesn't get
passed back through to the client.  If we're modifying the wire protocol to pass RPC version,
I think we should also take advantage of the change to serialize {{Status}} back to the client.
 This allows the client to differentiate between fatal errors (authentication error, or bad
rpc version) which should kill the connection, and request-specific errors.  By the way, adding
{{Status}} here allows me to remove it from the HBASE-2742 patch, which is great!

In {{Invocation}}:
# In the constructor:
    rpcVersion = WritableRpcEngine.writableRpcVersion;
This binds {{Invocation}} unnecessarily to {{WritableRpcEngine}}.  We've diverged a little
from Hadoop RPC by making {{Invocation}} a top-level class, allowing it to be shared between
RPC engine implementations, so this would undermine that.  Since {{rpcVersion}} only seems
to relate to {{Invocation}} serialization, why not just define {{RPC_VERSION}} as a static
final constant on {{Invocation}}?  Or alternately, couldn't we just make {{Invocation}} implement
{{VersionedWritable}} and let that handle the check for us?

Other than that, this patch looks fine to me.  I've applied it together with the HBASE-2742
patch and run through a few of the tests, and so far seems to work fine.
> Some crossports of Hadoop IPC fixes
> -----------------------------------
>                 Key: HBASE-3939
>                 URL: https://issues.apache.org/jira/browse/HBASE-3939
>             Project: HBase
>          Issue Type: Bug
>    Affects Versions: 0.92.0
>            Reporter: Todd Lipcon
>            Priority: Critical
>             Fix For: 0.92.0
>         Attachments: 3939-v2.txt, 3939-v3.txt, 3939-v4.txt, 3939-v5.txt, 3939-v6.txt,
3939-v7.txt, 3939.txt
> A few fixes from Hadoop IPC that we should probably cross-port into our copy:
> - HADOOP-7227: remove the protocol version check at call time
> - HADOOP-7146: fix a socket leak in server
> - HADOOP-7121: fix behavior when response serialization throws an exception
> - HADOOP-7346: send back nicer error response when client is using an out of date IPC

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira


View raw message