[
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13239783#comment-13239783
]
[email protected] commented on HBASE-5619:
------------------------------------------------------
bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. > src/main/proto/RegionAdmin.proto, line 87
bq. > <https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line87>
bq. >
bq. > Isn't the response currently a void? And isnt' flush async (IIRC).
If so, under what circumstance would we be able to fill out this response?
bq.
bq. Jimmy Xiang wrote:
bq. I was thinking to set it if the HRegion.flushCache call returned true.
It is just informational.
bq.
bq. Michael Stack wrote:
bq. Does that imply a synchronous call? I thought flushCache just queued
the flush then returned to the client w/o actually waiting on flush to complete.
I checked the code and it seems that flushCache is a synchronous call. Should
we make it async? If so, I can make flushed to queued.
bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. > src/main/proto/RegionAdmin.proto, line 70
bq. > <https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line70>
bq. >
bq. > Should we be repeating the API documentation that is up in the
HRegionInterface that this .proto replaces here? If its not here, where will
it be? Not all of the javadoc should make it over -- the stuff that says
nothing shouldn't but some is of worth. What you think?
bq.
bq. Jimmy Xiang wrote:
bq. I though about this. I added documentation to RegionClient.proto.
For methods in RegionAdmin, the method names seem to be very clear. I will
take a look again and document where confusing/misunderstand could arise.
bq.
bq. Michael Stack wrote:
bq. I'd be fine w/ the doc being sparse but there are a few cases where
doc is necessary; e.g. the one I cite above.
Sure, I will add more doc where confusion could happen.
bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. > src/main/proto/RegionAdmin.proto, line 35
bq. > <https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line35>
bq. >
bq. > What are these two booleans broken out? Aren't they in they
attributes of HRI already? Why repeat them?
bq.
bq. Jimmy Xiang wrote:
bq. I used to put these transient parameters in the protobuff RegionInfo
as well. However Todd thought it's better to put them outside.
bq.
bq. What'd do you think? To me, it is fine either way. However, if we
are going to replace HRI with the protobuff later on, it may be better to put
them together.
bq.
bq. Michael Stack wrote:
bq. Hmm. Moving out these flags changes the current 'model' but in a
direction we should be headed in. The split/offline stuff were stuffed into
HRI in the first place just because this was an easy way to pass these
transient states around in hbase; they also are less important now in HRI
though still depended on when we scan meta IIRC. Its probably better to evolve
toward an HRI that is immutable once made. So I'd be down w/ moving them out
but its up to you. It might be easier on you achieving parity w/ first commit
of pb work if the pb classes are like the internals they will be feeding into.
Ok, I can move them back. We can get back to this when time comes.
bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. > src/main/proto/RegionClient.proto, line 77
bq. > <https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line77>
bq. >
bq. > what is processed?
bq.
bq. Jimmy Xiang wrote:
bq. Sometimes, for some action, for example delete, they don't care about
the result. They just want to know if the action is processed or not. This is
for this kind use case.
bq.
bq. Michael Stack wrote:
bq. Needs comment
Will add comment
bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. > src/main/proto/RegionClient.proto, line 159
bq. > <https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line159>
bq. >
bq. > Again, does it make sense having this in here? I mean regions come
and go -- e.g. split -- so you could have reference to non-existent region.
This stuff of tying a mutation to a particular region should be done external
to the mutations themselves?
bq. >
bq. > Is this trying to do multiaction? Maybe that should be a message
apart from mutate? The new message would have region name and the mutate?
bq.
bq. Jimmy Xiang wrote:
bq. Similar to Gets, I'd like to make the interface flexible. Sounds you
prefer to have simple interface instead, right? That means we may have to have
too many methods. For java interface, it is ok and preferred to have many
methods. But for RPC, I think it is better to have fewer and flexible methods.
bq.
bq. Michael Stack wrote:
bq. 'But for RPC, I think it is better to have fewer and flexible
methods.' Fair enough. Would like to make it just flexible enough and no more
(smile). Needs documentation too else fellas like me will come in and get
wrong impression on whats going on.
Sure. I will make it simple for now. We can enhance if requirements come up.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review6365
-----------------------------------------------------------
On 2012-03-26 20:14:22, 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-03-26 20:14:22)
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-5619.
bq. https://issues.apache.org/jira/browse/HBASE-5619
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. pom.xml 10b13ef
bq. src/main/proto/RegionAdmin.proto PRE-CREATION
bq. src/main/proto/RegionClient.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.
> Create PB protocols for HRegionInterface
> ----------------------------------------
>
> Key: HBASE-5619
> URL: https://issues.apache.org/jira/browse/HBASE-5619
> Project: HBase
> Issue Type: Sub-task
> Components: ipc, master, migration, regionserver
> Reporter: Jimmy Xiang
> Assignee: Jimmy Xiang
> Fix For: 0.96.0
>
> Attachments: hbase-5619.patch, hbase-5619_v3.patch
>
>
> Subtask of HBase-5443, separate HRegionInterface into admin protocol and
> client protocol, create the PB protocol buffer files
--
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
