[
https://issues.apache.org/jira/browse/HBASE-5451?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13244006#comment-13244006
]
[email protected] commented on HBASE-5451:
------------------------------------------------------
bq. On 2012-04-02 00:21:20, Michael Stack wrote:
bq. > Some more questions. Just being careful DD.
That's fine. Hope the answers below are okay. Please let me know your response
soon so that I can submit another patch.
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/io/DataOutputOutputStream.java,
line 25
bq. > <https://reviews.apache.org/r/4096/diff/3/?file=97739#file97739line25>
bq. >
bq. > We should just be using the hadoop DOOS... looks like no diff (when
I diff them). I'll make an issue to remove.
cool
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 446
bq. > <https://reviews.apache.org/r/4096/diff/3/?file=97740#file97740line446>
bq. >
bq. > Is this written up anywhere? That its hrpc, then version, then a
length, then a protobuf?
bq. >
bq. > I see it in the proto definition. That'll do.
cool
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?
No not yet. But I'll create one to do with this issue once this patch is
committed.
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
Yeah, valid points. We can remove this version string and all in a follow up
patch.
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.
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.
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?
Actually, I could make this field 'optional' since this has a default value.
Will do so.
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?
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.
'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. 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?
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.
- Devaraj
-----------------------------------------------------------
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
