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:
{code}
    rpcVersion = WritableRpcEngine.writableRpcVersion;
{code}
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
version

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

        

Mime
View raw message