[
https://issues.apache.org/jira/browse/HBASE-5451?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13244255#comment-13244255
]
[email protected] 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/java/org/apache/hadoop/hbase/ipc/HBaseClient.java,
line 548
bq. > <https://reviews.apache.org/r/4096/diff/3/?file=97740#file97740line548>
bq. >
bq. > We have an issue for removing this Invocation stuff?
bq.
bq. Devaraj Das wrote:
bq. No not yet. But I'll create one to do with this issue once this patch
is committed.
Thanks
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 25
bq. > <https://reviews.apache.org/r/4096/diff/3/?file=97744#file97744line25>
bq. >
bq. > Should we just remove them in the next iteration on rpc since 0.96
is to be a singularity? Why even bother trying to keep compatibility w/ older
clients?
bq. >
bq. > What is 'failure compatibility'? We are telling the client to go
away, nicely (smile).
bq. >
bq. > What you think we should replace hrpc0x0005 with?
bq. >
bq. > this -> these
bq.
bq. Devaraj Das wrote:
bq. Yeah, valid points. We can remove this version string and all in a
follow up patch.
Lets discuss in another jira. A <hrpc> <version> followed by something that
says its protobuf that follows, etc.,
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 28
bq. > <https://reviews.apache.org/r/4096/diff/3/?file=97744#file97744line28>
bq. >
bq. > How does RpcRequestWithHeaderProto relate to ConnectionHeaderProto?
This text should say?
bq. >
bq. > Would be nice to have illustration on how the back and forth work.
bq.
bq. Devaraj Das wrote:
bq. The latter is used only while establishing connections and the former
for exchanging RPC requests/responses over a channel that is connected. Okay,
will add some text.
Thanks
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 55
bq. > <https://reviews.apache.org/r/4096/diff/3/?file=97744#file97744line55>
bq. >
bq. > We'll send this String each time?
bq.
bq. Devaraj Das wrote:
bq. Actually, I could make this field 'optional' since this has a default
value. Will do so.
That'd be a good idea I think. The other protocols are less used and can
include the String
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..
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?
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. 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.
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).
- 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
