Mailing-List: contact issues-help@hbase.apache.org; run by ezmlm
Precedence: bulk
Date: Mon, 2 Apr 2012 17:39:22 +0000 (UTC)
From: "jiraposter@reviews.apache.org (Commented) (JIRA)" <jira@apache.org>
To: issues@hbase.apache.org
Message-ID: 
 <982222577.1473.1333388362901.JavaMail.tomcat@hel.zones.apache.org>
In-Reply-To: 
 <758839952.3211.1329893569204.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] [Commented] (HBASE-5451) Switch RPC call envelope/headers to
 PBs
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

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


bq.  On 2012-04-02 00:21:20, Michael Stack wrote:
bq.  > http://svn.apache.org/repos/asf/hbase/trunk/src/main/protobuf/RPCMessageProto.proto, line 66
bq.  > <https://reviews.apache.org/r/4096/diff/3/?file=97744#file97744line66>
bq.  >
bq.  >     Which part in here is the 'header'?  How does it relate to ConnectionHeaderProto?
bq.  >     
bq.  >     request can be an Invocation/Writable?  Or a protobuf?  Do we need a length in here?
bq.  
bq.  Devaraj Das wrote:
bq.      Today the only 'header' is the callId.. There is no relation to ConnectionHeaderProto. If the 'header' is confusing, I can take it off the object name. Let me know.
bq.      
bq.      'request' in this patch is only a Invocation/Writable. In theory, it could be a protobuf object as well (since it is just bytes), but, for protobuf, we could make things more explicit by defining a protobuf object rather than a opaque set of bytes. But that's another jira (ProtoBufRpcEngine implementation similar to Hadoop). Length is not needed - the protobuf serialization/deserialization will take care of it..
bq.  
bq.  Michael Stack wrote:
bq.      I think taking the 'Header' off Request/Response would be best (Did I ask you add it previous?  If so, sorry... I misunderstood.  Thanks for being accomodating).   Yes, on a new issue to make it pb rather than opaque bytes.  Do you have to do something here -- make bytes optional? -- to allow for the later pb replacement?
bq.      
bq.      On length, thats probably good to keep.  For us, we'll give the stream to a pb deserializer but other clients might want to know how many bytes on the line.... so keep it I'd say.
bq.  
bq.  Devaraj Das wrote:
bq.      Yes, I'll take off the 'header' from the message name. I could make the 'bytes' field optional.
bq.      
bq.      Actually, on the length, I am not sure I understand why we need it in the PB model. Generally speaking, clients talking to servers have to be aware of the PB encoding in order for them to make any sense of the PB data.. The PB type 'bytes' has the length taken care of in the implementation of serialization/deserialization internally. In that sense, I don't think having an explicit length field is required. Does this reasoning make sense?
bq.      
bq.      (Also note that the top level RPC request envelope has the length preceding the request data)

If the top level rpc request envelope has the length, then I agree w/ you, its not needed as prefix on pb messages.


bq.  On 2012-04-02 00:21:20, Michael Stack wrote:
bq.  > http://svn.apache.org/repos/asf/hbase/trunk/src/main/protobuf/RPCMessageProto.proto, line 93
bq.  > <https://reviews.apache.org/r/4096/diff/3/?file=97744#file97744line93>
bq.  >
bq.  >     Should this precede the response?  So if false, a response follows else an exception?  Do we need a length here?  Where is the header that the message name refers too?
bq.  
bq.  Devaraj Das wrote:
bq.      Length will be taken care of by the protobuf serialization/deserialization. The header is the combination of callId, error. If the 'header' is confusing, I can take it off the object name. Let me know.
bq.  
bq.  Michael Stack wrote:
bq.      Yeah, take away the header.  Length I think is good.  Makes it more robust (IIRC, we went out of our way to add length to the old RPC to help clients figure how much to pull).
bq.  
bq.  Devaraj Das wrote:
bq.      The argument above for 'length' applies here too...

Agreed.  So high level, the response and request have a length of the total message?  If so, don't need it down inside preceeding pb messages.


- Michael


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


On 2012-03-30 23:29:32, 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-30 23:29:32)
bq.  
bq.  
bq.  Review request for Michael Stack and Benoit Sigoure.
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/src/main/java/org/apache/hadoop/hbase/io/DataOutputOutputStream.java 1307644 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseClient.java 1307644 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java 1307644 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RPCMessageProtos.java PRE-CREATION 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/java/org/apache/hadoop/hbase/security/User.java 1307644 
bq.    http://svn.apache.org/repos/asf/hbase/trunk/src/main/protobuf/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, rpc-proto.r5.txt
>
>


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