[
https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217697#comment-13217697
]
jirapos...@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-02-27 22:31:45, Todd Lipcon wrote:
bq. > here are some initial thoughts, let's make sure we get good discussion
on this before commit.
Of course. This is just the first draft.
bq. On 2012-02-27 22:31:45, Todd Lipcon wrote:
bq. > src/main/proto/HRegionProtocol.proto, lines 48-60
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line48>
bq. >
bq. > I think we could collapse Put and Delete
I'd like to. That means we can collapse both call put() and delete() too. If
so, what's a good name?
bq. On 2012-02-27 22:31:45, Todd Lipcon wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 83
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line83>
bq. >
bq. > typo
Will fix.
bq. On 2012-02-27 22:31:45, Todd Lipcon wrote:
bq. > src/main/proto/HRegionProtocol.proto, lines 88-102
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line88>
bq. >
bq. > we seem to be missing a row here.
Good catch. Will add it.
bq. On 2012-02-27 22:31:45, Todd Lipcon wrote:
bq. > src/main/proto/hbase.proto, line 27
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line27>
bq. >
bq. > isn't the regionName just the concatenation of other fields here?
Will remove it.
bq. On 2012-02-27 22:31:45, Todd Lipcon wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 323
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line323>
bq. >
bq. > why do we need a regioninfo here? we can locate the region by its
split point, can't we?
split point is an optional row. I think we do need regioninfo.
bq. On 2012-02-27 22:31:45, Todd Lipcon wrote:
bq. > src/main/proto/hbase.proto, lines 31-32
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line31>
bq. >
bq. > these two don't seem to belong here, since they're transient *state*
rather than properties of the region itself
Will move them somewhere else.
bq. On 2012-02-27 22:31:45, Todd Lipcon wrote:
bq. > src/main/proto/hbase.proto, line 38
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line38>
bq. >
bq. > when do you set allTime? isn't "allTime" the same as setting both of
the above to null?
Right. Will remove it.
bq. On 2012-02-27 22:31:45, Todd Lipcon wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 313
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line313>
bq. >
bq. > Can this be collapsed with CloseRegion?
bq. > Maybe we can have a proto like:
bq. > message RegionSpecifierProto {
bq. > required RegionSpecifierType type = 1;
bq. > required bytes data = 2;
bq. > enum RegionSpecifierType {
bq. > BY_REGION_ID_SHA1=0,
bq. > BY_CONTAINED_ROW=1,
bq. > BY_FULL_NAME=2
bq. > }
bq. > }
bq. > or something? Seems like it would be useful throughout to refer to
regions in places where it's nice to have flexibility
I can collapse it with closeRegion by making both regionInfo and
encodedRegionName optional.
bq. On 2012-02-27 22:31:45, Todd Lipcon wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 124
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line124>
bq. >
bq. > Why are the HLog-related things under HRegionProtocol? Seems like
these shouldn't be exposed to clients.
For management and replication related things, should I move them to a separate
interface, called HRegionMasterInterface, HRegionManagementInterface,
HManagementInterface, or something else?
We already have a HMasterRegionInterface.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5364
-----------------------------------------------------------
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
