[
https://issues.apache.org/jira/browse/HBASE-5445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13230709#comment-13230709
]
[email protected] commented on HBASE-5445:
------------------------------------------------------
bq. On 2012-03-13 05:33:05, Michael Stack wrote:
bq. > Looks good. Just a few comments below.
Thanks for the review. Will address in next patch.
bq. On 2012-03-13 05:33:05, Michael Stack wrote:
bq. > src/main/proto/HMasterProtocol.proto, line 85
bq. > <https://reviews.apache.org/r/4283/diff/1/?file=91002#file91002line85>
bq. >
bq. > 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.
I don't think they are used elsewhere, will check.
bq. On 2012-03-13 05:33:05, Michael Stack wrote:
bq. > src/main/proto/HMasterProtocol.proto, line 112
bq. > <https://reviews.apache.org/r/4283/diff/1/?file=91002#file91002line112>
bq. >
bq. > Is this class deprecated (HServerInfo?)
This is equivalent to HServerLoad.
bq. On 2012-03-13 05:33:05, Michael Stack wrote:
bq. > src/main/proto/hbase.proto, line 21
bq. > <https://reviews.apache.org/r/4283/diff/1/?file=91003#file91003line21>
bq. >
bq. > The package in first class is protobuf.generated. Here its
generated.protobuf. Did you mean that?
Good catch.
bq. On 2012-03-13 05:33:05, Michael Stack wrote:
bq. > src/main/proto/hbase.proto, line 119
bq. > <https://reviews.apache.org/r/4283/diff/1/?file=91003#file91003line119>
bq. >
bq. > This is ServerName? Why call it ServerSpecifier? It should be a
different name?
I was just matching it up so it would be consistent with RegionSpecifier --
I'll figure out something better for this.
bq. On 2012-03-13 05:33:05, Michael Stack wrote:
bq. > src/main/proto/HMasterProtocol.proto, line 22
bq. > <https://reviews.apache.org/r/4283/diff/1/?file=91002#file91002line22>
bq. >
bq. > Is this a good name for this class? Drop the H? Can we change it?
I'll drop the H everywhere I can :).
bq. On 2012-03-13 05:33:05, Michael Stack wrote:
bq. > src/main/proto/HMasterProtocol.proto, line 19
bq. > <https://reviews.apache.org/r/4283/diff/1/?file=91002#file91002line19>
bq. >
bq. > Reference your nice PDF?
bq. >
bq. > 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.
bq. >
bq. > 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?
bq. >
bq. > Otherwise, the pdf looks good.
I got rid of "public void assign(final byte [] regionName, final boolean
force)" which is already deprecated, so nothing to do for 0.94.
Do you have any recommendations for:
"public boolean balanceSwitch(final boolean b)" and "public boolean
synchronousBalanceSwitch(final boolean b)"?
which is what I combined into loadBalancerSwitchIs. I thought "switch" was
confusing because it is unclear if that is a verb (i.e. it switches whether the
load balancer is running, so non-idempotent), vs a noun (it is a switch like an
oven switch [on/off], so idempotent). I think a noun is the correct way to
think about it; I thought "SwitchIs" makes it clearer that its a noun. On
second though, I think I'd just call it "loadBalancerIs(final bool on,...)."
Should I add that method and deprecate the existing two methods above in 0.94?
I thought we were going to break everything in 0.96 anyway?
bq. On 2012-03-13 05:33:05, Michael Stack wrote:
bq. > src/main/proto/hbase.proto, line 149
bq. > <https://reviews.apache.org/r/4283/diff/1/?file=91003#file91003line149>
bq. >
bq. > Should ColumnMetadata and TableMetadata have common type?
They could be. Having them separate lets us evolve them independently, though.
- Gregory
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4283/#review5868
-----------------------------------------------------------
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
