hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "jiraposter@reviews.apache.org (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-5445) Add PB-based calls to HMasterInterface
Date Tue, 13 Mar 2012 05:33:51 GMT

    [ https://issues.apache.org/jira/browse/HBASE-5445?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13228224#comment-13228224
] 

jiraposter@reviews.apache.org 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

        

Mime
View raw message