[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217663#comment-13217663 ]
jirapos...@reviews.apache.org commented on HBASE-5443: ------------------------------------------------------ ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4054/#review5363 ----------------------------------------------------------- I notice you followed the same declaration order as HRegionInterface. Could we reorder these along say, functionality lines with appropriate comments in the .proto file? NOTE: I see Todd suggested splitting along management/client lines. That may be superior. Here's a sketch of what I think "along functionality lines" would look like: // Row(s) level get put delete mutate openScanner fetchFromScanner closeScanner lockRow unlockRow // Region level info getRegionInfo getOnlineRegions getLastFlushTime getStoreFileList // Region-level mutation flushRegion openRegion closeRegion closeRegionByEncodedName splitRegion compactRegion bulkLoadHFiles execCoprocessor // RegionServer-level getBlockCacheColumnFamilySummaries replicateLogEntry rollLogWriter stopServer src/main/proto/HRegionProtocol.proto <https://reviews.apache.org/r/4054/#comment11669> First enum is missing a comment. I might just get rid of the comments though, I don't think they really add anything. src/main/proto/HRegionProtocol.proto <https://reviews.apache.org/r/4054/#comment11687> The name mutate doesn't seem consistent to me. For one, "mutate" seems to not support Put/Deletes, but RowMutation *only* seems to support Puts/Deletes. Perhaps we can collapse this somehow? src/main/proto/HRegionProtocol.proto <https://reviews.apache.org/r/4054/#comment11670> this should be heapSize. src/main/proto/HRegionProtocol.proto <https://reviews.apache.org/r/4054/#comment11688> You got rid of mutateRow and suggest the new put/delete can support as along as there are no mixed puts and deletes. But it looks like mutateRow allowed you to do mixed puts and deletes atomically. Could you support atomic puts and deletes with what you have here? - Gregory On 2012-02-27 18:54:31, Jimmy Xiang wrote: bq. bq. ----------------------------------------------------------- bq. This is an automatically generated e-mail. To reply, visit: bq. https://reviews.apache.org/r/4054/ bq. ----------------------------------------------------------- bq. bq. (Updated 2012-02-27 18:54:31) bq. bq. bq. Review request for hbase. bq. bq. bq. Summary bq. ------- bq. bq. This is the first draft of the ProtoBuff HRegionProtocol. The corresponding java vs pb method mapping is attached to the jira: https://issues.apache.org/jira/browse/HBASE-5443 bq. bq. Please review. I'd like to move ahead after we get to some agreement. bq. bq. bq. This addresses bug HBASE-5443. bq. https://issues.apache.org/jira/browse/HBASE-5443 bq. bq. bq. Diffs bq. ----- bq. bq. pom.xml 066c027 bq. src/main/proto/HRegionProtocol.proto PRE-CREATION bq. src/main/proto/hbase.proto PRE-CREATION bq. bq. Diff: https://reviews.apache.org/r/4054/diff bq. bq. bq. Testing bq. ------- bq. bq. bq. Thanks, bq. bq. Jimmy bq. bq. > Add PB-based calls to HRegionInterface > -------------------------------------- > > Key: HBASE-5443 > URL: https://issues.apache.org/jira/browse/HBASE-5443 > Project: HBase > Issue Type: Sub-task > Components: ipc, master, migration, regionserver > Reporter: Todd Lipcon > Assignee: Jimmy Xiang > Fix For: 0.96.0 > > Attachments: region_java-proto-mapping.pdf > > -- 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