hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-5451) Switch RPC call envelope/headers to PBs
Date Mon, 26 Mar 2012 22:02:30 GMT

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

jiraposter@reviews.apache.org commented on HBASE-5451:
------------------------------------------------------



bq.  On 2012-03-24 07:38:03, Benoit Sigoure wrote:
bq.  > http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java,
line 102
bq.  > <https://reviews.apache.org/r/4096/diff/2/?file=86903#file86903line102>
bq.  >
bq.  >     Argh, no, don't change this!  I got other HBase devs to promise to not change
this as it makes backwards compatible clients impossibly complicated.

I see. This was the basis of the "graceful" failure for current clients that are not aware
of PB (clients would bail out if the versions of RPC don't match, right). The response to
your comment below "I don't see how this is graceful." is actually this change in the version.


bq.  On 2012-03-24 07:38:03, Benoit Sigoure wrote:
bq.  > http://svn.apache.org/repos/asf/hbase/trunk/src/main/proto/RPCMessageProto.proto,
line 34
bq.  > <https://reviews.apache.org/r/4096/diff/2/?file=86905#file86905line34>
bq.  >
bq.  >     Why keep this oddity of Hadoop RPC?  Either rely on TCP keepalive, or add a
Ping method to the RPC interface.

Note that this is just documentation. Ping is already done in hbase RPC, and I thought I'd
document it. I haven't done anything in the PB stuff for handling this. I agree with you this
is odd/special-case and IMO a topic for a separate jira.


bq.  On 2012-03-24 07:38:03, Benoit Sigoure wrote:
bq.  > http://svn.apache.org/repos/asf/hbase/trunk/src/main/proto/RPCMessageProto.proto,
line 72
bq.  > <https://reviews.apache.org/r/4096/diff/2/?file=86905#file86905line72>
bq.  >
bq.  >     Why is this optional?

General comment on the optional vs required PB fields... I have made most of the fields as
optional since it makes the specification flexible and makes compatibility very easy. Once
we are somewhat certain of the PB fields in the RPC we can finalize on the labeling of optional/required
on the fields. Does this make sense?


bq.  On 2012-03-24 07:38:03, Benoit Sigoure wrote:
bq.  > http://svn.apache.org/repos/asf/hbase/trunk/src/main/proto/RPCMessageProto.proto,
line 71
bq.  > <https://reviews.apache.org/r/4096/diff/2/?file=86905#file86905line71>
bq.  >
bq.  >     What's the point of this message?  Why not just put the callId in RpcRequestProto
and be done with it?

The main reason being I wanted to clearly separate what comes from the application and what's
put in by the RPC layer. The client would frame a PB object (RpcRequestProto) and send it
down to the RPC layer. Currently, the RpcRequestProto is mostly a placeholder with only one
field called 'bytes'. Once I implement the ProtoBufRpcEngine (as in Hadoop core) in a follow-up
jira, I'll have fields like "methodname', 'protocolname', etc. and they would be encoded as
RpcRequestProto objects.

Similarly, on the response side.


bq.  On 2012-03-24 07:38:03, Benoit Sigoure wrote:
bq.  > http://svn.apache.org/repos/asf/hbase/trunk/src/main/proto/RPCMessageProto.proto,
line 25
bq.  > <https://reviews.apache.org/r/4096/diff/2/?file=86905#file86905line25>
bq.  >
bq.  >     I don't see how this is graceful.

I answered this above.


- Devaraj


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4096/#review6302
-----------------------------------------------------------


On 2012-03-01 03:40:14, Devaraj Das wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/4096/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-03-01 03:40:14)
bq.  
bq.  
bq.  Review request for .
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Switch RPC call envelope/headers to PBs
bq.  
bq.  
bq.  This addresses bug HBASE-5451.
bq.      https://issues.apache.org/jira/browse/HBASE-5451
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    http://svn.apache.org/repos/asf/hbase/trunk/pom.xml 1294899 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/DataOutputOutputStream.java
1294899 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java
1294899 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java
1294899 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/User.java
1294899 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/proto/RPCMessageProto.proto PRE-CREATION

bq.  
bq.  Diff: https://reviews.apache.org/r/4096/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Devaraj
bq.  
bq.


                
> Switch RPC call envelope/headers to PBs
> ---------------------------------------
>
>                 Key: HBASE-5451
>                 URL: https://issues.apache.org/jira/browse/HBASE-5451
>             Project: HBase
>          Issue Type: Sub-task
>          Components: ipc, master, migration, regionserver
>    Affects Versions: 0.94.0
>            Reporter: Todd Lipcon
>            Assignee: Devaraj Das
>             Fix For: 0.96.0
>
>         Attachments: rpc-proto.2.txt, rpc-proto.3.txt, rpc-proto.patch.1_2
>
>


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