[
https://issues.apache.org/jira/browse/HBASE-5451?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13239989#comment-13239989
]
[email protected] 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/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?
bq.
bq. Devaraj Das wrote:
bq. 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.
bq.
bq. Similarly, on the response side.
bq.
bq.
bq. Michael Stack wrote:
bq. How hard to leave it out DD and add later if we need it?
I'll check on whether the current stuff can be moved into one PB cleanly (but
again in the near future we'll need to break it up into two as per my current
thinking of how things will be implemented).
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?
bq.
bq. Devaraj Das wrote:
bq. 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.
bq. Michael Stack wrote:
bq. Sure in general. What about the specific comment? Seems like its
required?
Ok.. I'll make the critical fields "REQUIRED"
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.
bq.
bq. Devaraj Das wrote:
bq. 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.
bq. Michael Stack wrote:
bq. Benoit's point is that this mechanism doesn't work so his point is
lets not bother changing the version. Previous, if you volunteered a hrpc
version other than what is expected, the connection was closed by the server
w/o saying what was wrong. We fixed hbase so it at least throws an exception
but it doesn't say what version its expecting.
Stack, if we don't change the server version number then even the exception
you're referring to won't be thrown. The exception/error will happen later on
in the processing of the RPC... Are we sure we want this as the behavior?
Please let me know.
- 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
