[
https://issues.apache.org/jira/browse/HBASE-5445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13228224#comment-13228224
]
[email protected] commented on HBASE-5445:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4283/#review5868
-----------------------------------------------------------
Looks good. Just a few comments below.
src/main/proto/HMasterProtocol.proto
<https://reviews.apache.org/r/4283/#comment12790>
Reference your nice PDF?
BTW, in your PDF, you have one method that is 'not used' so you don't
include it. Do we need to deprecate it in 0.94 so its safe to remove by the
time 0.96 comes around? If so, mind filing an issue or just me know and I can
take care of it.
Ditto for loadBalancerSwitchIs(LoadBalancerSwitchIsRequest). This method
is just broke from its name and to how it works. Can we deprecate it in 0.94?
Add a method that makes sense in 0.94, one that makes sense that you can redo
in pb?
Otherwise, the pdf looks good.
src/main/proto/HMasterProtocol.proto
<https://reviews.apache.org/r/4283/#comment12791>
Did Benoit comment that protobuf is too long, just do pb (Ask Jimmy? He'd
remember)
src/main/proto/HMasterProtocol.proto
<https://reviews.apache.org/r/4283/#comment12792>
Is this a good name for this class? Drop the H? Can we change it?
src/main/proto/HMasterProtocol.proto
<https://reviews.apache.org/r/4283/#comment12793>
These are in the master protocol but are they used elsewhere? If so,
should they be in here? Maybe they are not and its just me confused.
src/main/proto/HMasterProtocol.proto
<https://reviews.apache.org/r/4283/#comment12794>
Is this class deprecated (HServerInfo?)
src/main/proto/HMasterProtocol.proto
<https://reviews.apache.org/r/4283/#comment12795>
Yeah, this method is broke as said above. We should try do patch up work
in 0.94 so you can come in all clean and shiny in 0.96.
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4283/#comment12796>
The package in first class is protobuf.generated. Here its
generated.protobuf. Did you mean that?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4283/#comment12797>
What is one of these? A bit of a comment?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4283/#comment12798>
ditto. Whats this map too?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4283/#comment12799>
We used to 'guess' this from the bytes passed. You are changing that?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4283/#comment12800>
And we'd generate the encoded name from these articles?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4283/#comment12801>
uint32 seems wide for number of stores
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4283/#comment12802>
ditto
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4283/#comment12803>
This is ServerName? Why call it ServerSpecifier? It should be a different
name?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4283/#comment12804>
Should ColumnMetadata and TableMetadata have common type?
- Michael
On 2012-03-10 02:09:45, Gregory Chanan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/4283/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-03-10 02:09:45)
bq.
bq.
bq. Review request for hbase.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. First draft of the protobufs specification for HMasterInterface.
bq. This is relatively close to a one-to-one mapping with the existing
interface. A pdf listing the existing-to-protobufs mapping is available on the
JIRA: https://issues.apache.org/jira/browse/HBASE-5445
bq.
bq. Thanks to Devaraj who provided some initial work he had done on this.
bq.
bq.
bq. This addresses bug HBASE-5445.
bq. https://issues.apache.org/jira/browse/HBASE-5445
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. src/main/proto/HMasterProtocol.proto PRE-CREATION
bq. src/main/proto/hbase.proto PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/4283/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Gregory
bq.
bq.
> Add PB-based calls to HMasterInterface
> --------------------------------------
>
> Key: HBASE-5445
> URL: https://issues.apache.org/jira/browse/HBASE-5445
> Project: HBase
> Issue Type: Sub-task
> Components: ipc, master, migration, regionserver
> Reporter: Todd Lipcon
> Assignee: Gregory Chanan
> Fix For: 0.96.0
>
> Attachments: HMasterInterfaceMappingv0.pdf, HMasterProtocol.proto,
> hbase.proto
>
>
--
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
