You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Todd Lipcon (Created) (JIRA)" <ji...@apache.org> on 2012/02/22 07:48:54 UTC
[jira] [Created] (HBASE-5443) Add PB-based calls to
HRegionInterface
Add PB-based calls to HRegionInterface
--------------------------------------
Key: HBASE-5443
URL: https://issues.apache.org/jira/browse/HBASE-5443
Project: HBase
Issue Type: Sub-task
Reporter: Todd Lipcon
Assignee: Jimmy Xiang
--
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217698#comment-13217698 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-02-27 22:54:55, Gregory Chanan wrote:
bq. > 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?
bq. > 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:
bq. >
bq. > // Row(s) level
bq. > get
bq. > put
bq. > delete
bq. > mutate
bq. > openScanner
bq. > fetchFromScanner
bq. > closeScanner
bq. > lockRow
bq. > unlockRow
bq. >
bq. > // Region level info
bq. > getRegionInfo
bq. > getOnlineRegions
bq. > getLastFlushTime
bq. > getStoreFileList
bq. >
bq. > // Region-level mutation
bq. > flushRegion
bq. > openRegion
bq. > closeRegion
bq. > closeRegionByEncodedName
bq. > splitRegion
bq. > compactRegion
bq. > bulkLoadHFiles
bq. > execCoprocessor
bq. >
bq. > // RegionServer-level
bq. > getBlockCacheColumnFamilySummaries
bq. > replicateLogEntry
bq. > rollLogWriter
bq. > stopServer
bq. >
Good idea. Will do.
bq. On 2012-02-27 22:54:55, Gregory Chanan wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 88
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line88>
bq. >
bq. > 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?
Yes. I think so. I used to collapse both put and delete to one call putOrDelete. But it sounds not very good to me. If we collapse them, what do you want it to be called?
bq. On 2012-02-27 22:54:55, Gregory Chanan wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 121
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line121>
bq. >
bq. > this should be heapSize.
Will fix.
bq. On 2012-02-27 22:54:55, Gregory Chanan wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 402
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line402>
bq. >
bq. > You got rid of mutateRow and suggest the new put/delete can support as along as there are no mixed puts and deletes.
bq. >
bq. > 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?
If we collapse put and delete, it should be supported.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5363
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221085#comment-13221085 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > This seems to be close to a one-to-one mapping with the current interface today. I don't know if this is the intent or whether you're willing to completely redesign the look of the API too. Maybe it's to ease the transition?
bq. >
bq. > I'd like to see a request type to do one-shot scans. Something where you don't even get a scanner ID. You pass parameters like to open a scanner, you say up to how many rows or bytes you want to retrieve, and you get just that in one shot.
bq. > When opening a actual scanner, we also need to be able to get the first batch of scan results at the same time we open the scanner. This is a must-have IMO. And we need to be able to request to close the scanner while fetching a batch of results.
bq. >
bq. > It would be nice to have a "keep-alive" request for existing scanners. Something to tell the server "I'm not fetching anything from this scanner right now, but please keep it open by reseting its TTL, don't close it just because I haven't used it for a while".
bq. >
bq. > Please, please, please, consider shortening the name of all these protobufs and dropping the Proto suffix. The current names are unnecessarily long or aren't intuitive (e.g. "columnFamily" for something that describes the multiple things you're trying to get out of a row) or are too redundant (e.g. "KeyType keyType").
bq. >
bq. > Regarding the lack of "multi" RPC, I think this is a good thing. "multi" is a big mess that was only marginally better than its horrible "multiPut" predecessor. This proposal already supports multi-everything, it just doesn't support batching different kind of operations in the same RPC, which isn't a big deal IMO.
We should implement what Benoît is asking for, probably not all as part of this issue. That said, if possible can we try and accomodate what he's asking for down here at the rpc level? I suppose once all is pb, it should be easy enough adding new stuff but it would be good to keep in mind what he's asking while redoing this layer. In a later issue we can add the overloads that exploit the additions or add the new methods B wants (What B is asking for are long-time outstanding fixups needed in hbase). For example, can the pb response on open of a scanner be more than just the scanner id; could it include an optional result item? Or I suppose, once up on pb, we can do this easily enough later?
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5552
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13243954#comment-13243954 ]
Jimmy Xiang commented on HBASE-5443:
------------------------------------
The main reason is that the HBase writable RPC already supports pb. Hadoop uses pb too.
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221238#comment-13221238 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > pom.xml, line 750
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86002#file86002line750>
bq. >
bq. > Do this instead:
bq. >
bq. > if which cygpath >/dev/null 2>/dev/null; then
bq. > # Windows
bq. > else
bq. > # Not Windows
bq. > fi
Why do we need /dev/null twice?
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > pom.xml, line 761
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86002#file86002line761>
bq. >
bq. > Actually you can just remove the whole $IS_WIN business and everything. Simply fix PROTO_DIR and JAVA_DIR when on Windows before calling protoc.
How about this? The reason for I_PROTO_DIR is to keep "ls $PROTO_DIR/*.proto" working.
if which cygpath 2> /dev/null; then
I_PROTO_DIR=$PROTO_DIR
else
I_PROTO_DIR=`cygpath --windows $PROTO_DIR`
JAVA_DIR=`cygpath --windows $JAVA_DIR`
fi
mkdir -p $JAVA_DIR 2> /dev/null
for PROTO_FILE in `ls $PROTO_DIR/*.proto 2> /dev/null`
do
protoc -I$I_PROTO_DIR --java_out=$JAVA_DIR $PROTO_FILE
done
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5552
-----------------------------------------------------------
On 2012-03-02 18:54:29, 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-02 18:54:29)
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 bb518b1
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.
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13218601#comment-13218601 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5407
-----------------------------------------------------------
High-level, do we need to split the Interface more -- into admin vs user protos? Would love to get rid of the plethora of methods but probably not a task to be done as part of this issue?
pom.xml
<https://reviews.apache.org/r/4054/#comment11742>
There are prereqs for this to work? I suppose compile-proto.sh checks and its failing it seems fails the build.. thats good.
pom.xml
<https://reviews.apache.org/r/4054/#comment11741>
Do we do this elsewhere in the pom? If so, should set a boolean once?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11743>
So, all protobuf files go here? We need to ensure this the case w/ all patches (I think the rpc patch was putting them elsewhere.....)
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11744>
Elsewhere in hbase the convention is 'what' and then the generated classes are in a 'generated' subpackage as in there is a thrift subpackage and under there are thrift utils and then a 'generated' subpackage. Ditto avro. This is different in that we have a generated top level subpackage w/ proto as a subpackage. Should we flip it?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11745>
Should the suffix be Proto -- our convention for Protobuf classes (?) -- or Protos? Why the plural? Perhaps this a container for a bunch of Proto?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11755>
This class seems to have more than HR stuff in it. Should we make more protos? A client proto?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11751>
Mutation (Probably already used)
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11752>
What is this? For filters?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11753>
Mutate is not deprecated? We don't have deprecated stuff in this proto file? This is doing what from current API?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11756>
Are these client datastructures? Or they go w/ the RS proto and are used by clients?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11754>
Where does this come from in current API?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11758>
Doesn't HRI have a tablename in it? So maybe this should have a HRI?
What is this LogKeyProto modeling? Should be WALKeyProto?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11759>
WALEntryProto?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11760>
Should there be more in here?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11761>
Is this a class or method name? If method name is convention to capitalize?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11762>
So, even if method returns a HRegionInfo, we return a GetRegionInfoResponseProto in case the response changes?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11763>
regionName and encodedName and HRegionInfo class.... should we standardize?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11764>
Excellent. Later we need to return first set of results in here rather than have to have client go back to do a next.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11765>
Maybe NextOnScannerRequestProto so relates better to our current API
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11766>
We should make this optional some day.. that you have to call a close.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11767>
Man. Anyone use this thing?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11768>
We should have one or the other. Can have one call through to the other (thats implementation?)
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11769>
Oh, I see how method naming and classes goes
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11770>
I wonder if these could take a more primitive type... a KV type.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11771>
nextOnScanner?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11772>
These are admin interface functions. Should we split the interface?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11773>
Yeah, we need to doc. this later. Ain't the doc here going to be our Interface doc? Our only Interface doc?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment11750>
Doesn't an HRI have a Map?
There does not seem to be enough in here.
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment11748>
My guess is that there is no uint32?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment11746>
Whats a 'name'? Is it a region name?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment11747>
Should this be uint32? (Maybe not possible in proto?)
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment11749>
I think a bit of doc in here would not go amiss. Else its hard to figure all is going on in here; what goes where.
Should ServerName be in this proto file?
- Michael
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221228#comment-13221228 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5583
-----------------------------------------------------------
src/main/proto/RegionAdmin.proto
<https://reviews.apache.org/r/4054/#comment12092>
If we are getting rid of "Proto" in the message names, might as well get rid of it here too.
src/main/proto/RegionClient.proto
<https://reviews.apache.org/r/4054/#comment12094>
Here too.
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment12095>
Here too.
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment12091>
You don't have "option optimize_for = SPEED;" here.
- Gregory
On 2012-03-02 18:54:29, 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-02 18:54:29)
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 bb518b1
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.
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "binlijin (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13243948#comment-13243948 ]
binlijin commented on HBASE-5443:
---------------------------------
Hi guys,i have some question, why choose pb? why not avro or thrift?
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217647#comment-13217647 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5364
-----------------------------------------------------------
here are some initial thoughts, let's make sure we get good discussion on this before commit.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11671>
I think we could collapse Put and Delete
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11672>
typo
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11673>
we seem to be missing a row here.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11674>
Why are the HLog-related things under HRegionProtocol? Seems like these shouldn't be exposed to clients.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11675>
would be nice to extract these "management interface" things from this protocol -- so the stuff that the average client needs goes in one place, and the stuff that only management tools would use go in another.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11676>
Also these calls which are supposed to only be called by the master, maybe we can move elsewhere
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11677>
Can this be collapsed with CloseRegion?
Maybe we can have a proto like:
message RegionSpecifierProto {
required RegionSpecifierType type = 1;
required bytes data = 2;
enum RegionSpecifierType {
BY_REGION_ID_SHA1=0,
BY_CONTAINED_ROW=1,
BY_FULL_NAME=2
}
}
or something? Seems like it would be useful throughout to refer to regions in places where it's nice to have flexibility
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11678>
why do we need a regioninfo here? we can locate the region by its split point, can't we?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment11680>
isn't the regionName just the concatenation of other fields here?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment11679>
these two don't seem to belong here, since they're transient *state* rather than properties of the region itself
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment11681>
when do you set allTime? isn't "allTime" the same as setting both of the above to null?
- Todd
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13220218#comment-13220218 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-03-01 18:23:27, Michael Stack wrote:
bq. > pom.xml, line 746
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86002#file86002line746>
bq. >
bq. > I don't see this script in this patch
This script is generated by the echo command here. The content of the script is inside the pom after this line.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5503
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217442#comment-13217442 ]
Jimmy Xiang commented on HBASE-5443:
------------------------------------
We can still support multi(MultiAction). Should we still support it in the RPC layer?
Can we put some logic in the client side, like aggregating the actions based
on region, action type (put/delete/get), and so on?
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13220221#comment-13220221 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5505
-----------------------------------------------------------
pom.xml
<https://reviews.apache.org/r/4054/#comment11926>
Duh. Thanks Jimmy. What about the defines? Where are we going to generate the files into?
- Michael
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13220238#comment-13220238 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-03-01 18:33:53, Michael Stack wrote:
bq. > pom.xml, line 746
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86002#file86002line746>
bq. >
bq. > Duh. Thanks Jimmy. What about the defines? Where are we going to generate the files into?
This compile-proto.sh is under target.
Those pb files are under target/generated-sources/java/org/apache/hadoop/hbase/proto/generated
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5505
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "stack (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13244522#comment-13244522 ]
stack commented on HBASE-5443:
------------------------------
There is also this write up of Todd's on why pb in first place over in hdfs: https://issues.apache.org/jira/browse/HDFS-2058?focusedCommentId=13047289&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13047289
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221218#comment-13221218 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-03-02 19:31:46, Jimmy Xiang wrote:
bq. > This is another option for scan. This way, we will have only one scan method, no need to open/next/close.
bq. >
bq. > Which one do you prefer? In the ScanRequest, either scannerId or scan must be specified, not both.
bq. >
bq. > message Scan {
bq. > required RegionSpecifier region = 1;
bq. > repeated Column column = 2;
bq. > repeated Attribute attribute = 3;
bq. > optional bytes startRow = 4;
bq. > optional bytes stopRow = 5;
bq. > optional string filterName = 6;
bq. > optional TimeRange timeRange = 7;
bq. > optional uint32 maxVersions = 8 [default = 1];
bq. > optional bool cacheBlocks = 9 [default = true];
bq. > optional uint32 rowsToCache = 10;
bq. > optional uint32 batchSize = 11;
bq. > }
bq. >
bq. > message ScanRequest {
bq. > optional uint64 scannerId = 1;
bq. > optional Scan scan = 2;
bq. > optional uint32 numberOfRows = 3;
bq. > optional bool closeScanner = 4;
bq. > optional uint32 ttl = 5;
bq. > }
bq. >
bq. > message ScanResponse {
bq. > repeated Result result = 1;
bq. > optional uint64 scannerId = 2;
bq. > optional bool moreResults = 3;
bq. > optional uint32 ttl = 4;
bq. > }
bq. >
bq.
bq. Michael Stack wrote:
bq. So we would do away with openScanner, next, and close, just do scan? Inside in the ScanRequest, we'd carry over the Scan specification each time? We'd be able to honor the current openScanner, next, close client-facing API but could add a new scan method to the public api that allowed passing the above specifications? Sounds good.
The only issue is that both optional. They need to know to specify one. From documentation?
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5575
-----------------------------------------------------------
On 2012-03-02 18:54:29, 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-02 18:54:29)
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 bb518b1
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.
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "binlijin (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13246138#comment-13246138 ]
binlijin commented on HBASE-5443:
---------------------------------
@Jimmy Xiang and @stack ,
Thank you very much!
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221119#comment-13221119 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > This seems to be close to a one-to-one mapping with the current interface today. I don't know if this is the intent or whether you're willing to completely redesign the look of the API too. Maybe it's to ease the transition?
bq. >
bq. > I'd like to see a request type to do one-shot scans. Something where you don't even get a scanner ID. You pass parameters like to open a scanner, you say up to how many rows or bytes you want to retrieve, and you get just that in one shot.
bq. > When opening a actual scanner, we also need to be able to get the first batch of scan results at the same time we open the scanner. This is a must-have IMO. And we need to be able to request to close the scanner while fetching a batch of results.
bq. >
bq. > It would be nice to have a "keep-alive" request for existing scanners. Something to tell the server "I'm not fetching anything from this scanner right now, but please keep it open by reseting its TTL, don't close it just because I haven't used it for a while".
bq. >
bq. > Please, please, please, consider shortening the name of all these protobufs and dropping the Proto suffix. The current names are unnecessarily long or aren't intuitive (e.g. "columnFamily" for something that describes the multiple things you're trying to get out of a row) or are too redundant (e.g. "KeyType keyType").
bq. >
bq. > Regarding the lack of "multi" RPC, I think this is a good thing. "multi" is a big mess that was only marginally better than its horrible "multiPut" predecessor. This proposal already supports multi-everything, it just doesn't support batching different kind of operations in the same RPC, which isn't a big deal IMO.
bq.
bq. Michael Stack wrote:
bq. We should implement what Benoît is asking for, probably not all as part of this issue. That said, if possible can we try and accomodate what he's asking for down here at the rpc level? I suppose once all is pb, it should be easy enough adding new stuff but it would be good to keep in mind what he's asking while redoing this layer. In a later issue we can add the overloads that exploit the additions or add the new methods B wants (What B is asking for are long-time outstanding fixups needed in hbase). For example, can the pb response on open of a scanner be more than just the scanner id; could it include an optional result item? Or I suppose, once up on pb, we can do this easily enough later?
bq.
bq.
The idea is not to break the existing client application code. So the new interface should be able to do the same thing and more.
By the way, I have changed the interfaces a lot after several reviews so I closed this review. I will post a new review later.
As to scanner, we cannot retrieve everything in one shot. So in the RPC layer, there must be multiple trips. As to the function you mentioned, it can be built in the client side, right?
I will add an option to return results in opening a scanner, and an option to close the scanner in fetching from the scanner.
Ok, I will try to shorten the names.
As to multi, I am not sure. This proposal doesn't support mix different kind of operations in different order in the same RPC. I may need to add a similar one if we don't want to break the existing function.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 22
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line22>
bq. >
bq. > I find the Proto suffix unnecessary and long. If you truly want a suffix, PB would be shorter, but no suffix would be better IMO.
Ok, I will remove the Proto suffix.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 25
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line25>
bq. >
bq. > Use "option optimize_for = SPEED", it makes a big difference.
Cool. I will add it.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 28
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line28>
bq. >
bq. > I'd call this just "Columns".
I changed it to ColumnProto.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 30
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line30>
bq. >
bq. > I would recommend to pluralize all "repeated" fields. This will make for nicer code where you'll be able to write something along the lines of:
bq. >
bq. > for (byte[] qualifier : pb.qualifiers())
For the repeated fields, the generated code will have method addQualifier(index), getQualifierList(). I don't think we need pluralization.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 88
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line88>
bq. >
bq. > The thing that append() and increment() have in common is that they're atomic operations that don't require a read-modify-write from the client. So maybe AtomicOp would be better?
In my new one, I have an optional parameter to indicate if atomic operations requested, which is for a set of mutations.
One mutation such as append() and increment() should always be atomic, right?
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 192
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line192>
bq. >
bq. > What's the meaning of this? How do we know what has been processed and what hasn't?
I removed it.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 221
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line221>
bq. >
bq. > We need to have a way to get feedback from the server about the TTL of the scanner. How long can the client hold on to the scanner before the server will kill it. Add a field here so that the server can communicate the TTL to the client.
Sure. Will add it.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 226
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line226>
bq. >
bq. > Please add an "optional boolean close" to request that the scanner be closed after returning this batch of results. This can help clients eliminate the "CloseScannerRequestProto" when they know they're going to close the scanner after this batch.
Sure, will do.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 269
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line269>
bq. >
bq. > This name is inconsistent with the name of the request. By your scheme it should be named "LockRowResponseProto" – although I'd much prefer "LockResp" or something like that.
How about LockRowResponse?
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 271
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line271>
bq. >
bq. > This needs to have a field specifying how long the server is willing to hand out the lock for.
Ok, will add one.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/hbase.proto, line 36
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line36>
bq. >
bq. > Call these fields just "from" and "to".
Ok.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/hbase.proto, line 60
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line60>
bq. >
bq. > Why are all these fields optional? How can a KeyValue not have a family or a qualifier or a timestamp?
In case later on we replace the existing KeyValue class with this one.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/hbase.proto, line 62
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line62>
bq. >
bq. > I'd recommend naming this "timestamp" not "time".
ok.
bq. On 2012-03-02 10:30:06, Benoit Sigoure wrote:
bq. > src/main/proto/hbase.proto, line 63
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line63>
bq. >
bq. > Just call this "type".
If later on, we add another type, such as ValueType, it will be confusing, right?
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5552
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13218706#comment-13218706 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > High-level, do we need to split the Interface more -- into admin vs user protos? Would love to get rid of the plethora of methods but probably not a task to be done as part of this issue?
Yes. Todd mentioned that too. Will do.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/hbase.proto, line 36
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line36>
bq. >
bq. > My guess is that there is no uint32?
There is uint32. Should we use uint32 for timestamp? If it is in second, it is ok for many years. If it is in millisecond, we should use uint64.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/hbase.proto, line 42
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line42>
bq. >
bq. > Whats a 'name'? Is it a region name?
It is for a generic NameValue pair.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > pom.xml, line 749
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86002#file86002line749>
bq. >
bq. > Do we do this elsewhere in the pom? If so, should set a boolean once?
This is the only place.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 1
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line1>
bq. >
bq. > So, all protobuf files go here? We need to ensure this the case w/ all patches (I think the rpc patch was putting them elsewhere.....)
All proto files should go here, while Java files go to other places like other Java classes.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 21
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line21>
bq. >
bq. > Elsewhere in hbase the convention is 'what' and then the generated classes are in a 'generated' subpackage as in there is a thrift subpackage and under there are thrift utils and then a 'generated' subpackage. Ditto avro. This is different in that we have a generated top level subpackage w/ proto as a subpackage. Should we flip it?
Sure, will do.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 22
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line22>
bq. >
bq. > Should the suffix be Proto -- our convention for Protobuf classes (?) -- or Protos? Why the plural? Perhaps this a container for a bunch of Proto?
Yes, there are a bunch of messages. Each message is a Proto.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 23
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line23>
bq. >
bq. > This class seems to have more than HR stuff in it. Should we make more protos? A client proto?
Ok.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 62
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line62>
bq. >
bq. > What is this? For filters?
For checkAndPut, checkAndDelete, this is condition to check.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 88
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line88>
bq. >
bq. > Mutate is not deprecated? We don't have deprecated stuff in this proto file? This is doing what from current API?
This Mutate is different from the mutateRow() in HRegionInterface. It is for append() and increment(). Any suggestion for a better naming?
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 105
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line105>
bq. >
bq. > Are these client datastructures? Or they go w/ the RS proto and are used by clients?
For now, I prefer not to change the client data structures. So these go w/ the RS proto only and are not used by clients directly.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 118
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line118>
bq. >
bq. > Where does this come from in current API?
Yes.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 148
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line148>
bq. >
bq. > WALEntryProto?
Ok.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 126
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line126>
bq. >
bq. > Doesn't HRI have a tablename in it? So maybe this should have a HRI?
bq. >
bq. > What is this LogKeyProto modeling? Should be WALKeyProto?
WAL log key. Will change the name.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 162
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line162>
bq. >
bq. > Should there be more in here?
It should be bytes. I will it.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 165
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line165>
bq. >
bq. > Is this a class or method name? If method name is convention to capitalize?
It is the request class.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 169
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line169>
bq. >
bq. > So, even if method returns a HRegionInfo, we return a GetRegionInfoResponseProto in case the response changes?
That's right. The response proto contains the HRegionInfo in this case.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 174
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line174>
bq. >
bq. > regionName and encodedName and HRegionInfo class.... should we standardize?
Yes, working on it.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 223
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line223>
bq. >
bq. > Maybe NextOnScannerRequestProto so relates better to our current API
Sure. Will rename it.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 248
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line248>
bq. >
bq. > Man. Anyone use this thing?
Could not find any reference. Should we deprecate it?
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 408
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line408>
bq. >
bq. > nextOnScanner?
Will rename it.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 420
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line420>
bq. >
bq. > These are admin interface functions. Should we split the interface?
Yes, working on it now.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/hbase.proto, line 62
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line62>
bq. >
bq. > Should this be uint32? (Maybe not possible in proto?)
Is it better to use uint64? I assume it is in millisecond.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/hbase.proto, line 65
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line65>
bq. >
bq. > I think a bit of doc in here would not go amiss. Else its hard to figure all is going on in here; what goes where.
bq. >
bq. > Should ServerName be in this proto file?
Yes, ServerName should be in this file.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/hbase.proto, line 33
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86004#file86004line33>
bq. >
bq. > Doesn't an HRI have a Map?
bq. >
bq. > There does not seem to be enough in here.
I did not in any map in HRegionInfo. Anything specific I missed?
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 464
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line464>
bq. >
bq. > Yeah, we need to doc. this later. Ain't the doc here going to be our Interface doc? Our only Interface doc?
Right. Doc comes later.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 399
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line399>
bq. >
bq. > I wonder if these could take a more primitive type... a KV type.
Then we may have problem to enhance it, for example, add/remove a parameter. In this case, we can always add/deprecate optional parameters to the Request.
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 315
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line315>
bq. >
bq. > We should have one or the other. Can have one call through to the other (thats implementation?)
Will collapse closeRegionByName into closeRegion.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5407
-----------------------------------------------------------
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
[jira] [Updated] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Jimmy Xiang (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jimmy Xiang updated HBASE-5443:
-------------------------------
Attachment: region_java-proto-mapping.pdf
In the attached region_java-proto-mapping.pdf file, I have the first draft of proto rpc methods to the java interface mapping.
Please review, especially the following questions:
1. Should we combine put and delete to one method? Their signatures are kind of the same.
2. mutateRow(regionName, RowMutations)
For multiple puts or deletes, the new put/delete can support as long as there are no mixed puts and deletes.
Do we have to support mixed puts and deletes in RPC? Can we separate them in HBaseClient?
3. multi(MultiAction)
This one is too complicated. I think we should not put this in RPC. We can separate them in HBaseClient
and call corresponding puts/deletes/gets?
> 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
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13242824#comment-13242824 ]
Hudson commented on HBASE-5443:
-------------------------------
Integrated in HBase-TRUNK #2701 (See [https://builds.apache.org/job/HBase-TRUNK/2701/])
HBASE-5443 Create PB protocols for HRegionInterface (Revision 1307625)
Result = SUCCESS
stack :
Files :
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/HRegionPartitioner.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionAdminProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/package.html
* /hbase/trunk/src/main/protobuf
* /hbase/trunk/src/main/protobuf/README.txt
* /hbase/trunk/src/main/protobuf/RegionAdmin.proto
* /hbase/trunk/src/main/protobuf/RegionClient.proto
* /hbase/trunk/src/main/protobuf/hbase.proto
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217697#comment-13217697 ]
jiraposter@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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "binlijin (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13243950#comment-13243950 ]
binlijin commented on HBASE-5443:
---------------------------------
Hi guys,i have some question, why choose pb? why not avro or thrift?
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13254000#comment-13254000 ]
Hudson commented on HBASE-5443:
-------------------------------
Integrated in HBase-TRUNK-security #171 (See [https://builds.apache.org/job/HBase-TRUNK-security/171/])
HBASE-5443 Convert the client protocol of HRegionInterface to PB (Revision 1325937)
Result = FAILURE
stack :
Files :
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/filter/ParseConstants.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/AdminProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ClientProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionAdminProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java
* /hbase/trunk/src/main/protobuf/Admin.proto
* /hbase/trunk/src/main/protobuf/Client.proto
* /hbase/trunk/src/main/protobuf/RegionAdmin.proto
* /hbase/trunk/src/main/protobuf/RegionClient.proto
* /hbase/trunk/src/main/protobuf/hbase.proto
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.java
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217707#comment-13217707 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5372
-----------------------------------------------------------
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11701>
oh, right... here's another place we could use HRegionSpecifier or something, then?
- Todd
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13218738#comment-13218738 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-02-28 22:59:39, Shaneal Manek wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 84
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line84>
bq. >
bq. > What is this 'value' field? What is its serialization format?
This value field is the value used to compare. It is a plain byte array.
bq. On 2012-02-28 22:59:39, Shaneal Manek wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 28
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line28>
bq. >
bq. > Why is this called 'ColumnFamilyMapProto' (as opposed to just, say, 'ColumnProto' or 'ColumnNameProto')?
It is a column family and a list of its columns. ColumnListProto, ColumnsProto?
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5415
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221198#comment-13221198 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-03-02 19:31:46, Jimmy Xiang wrote:
bq. > This is another option for scan. This way, we will have only one scan method, no need to open/next/close.
bq. >
bq. > Which one do you prefer? In the ScanRequest, either scannerId or scan must be specified, not both.
bq. >
bq. > message Scan {
bq. > required RegionSpecifier region = 1;
bq. > repeated Column column = 2;
bq. > repeated Attribute attribute = 3;
bq. > optional bytes startRow = 4;
bq. > optional bytes stopRow = 5;
bq. > optional string filterName = 6;
bq. > optional TimeRange timeRange = 7;
bq. > optional uint32 maxVersions = 8 [default = 1];
bq. > optional bool cacheBlocks = 9 [default = true];
bq. > optional uint32 rowsToCache = 10;
bq. > optional uint32 batchSize = 11;
bq. > }
bq. >
bq. > message ScanRequest {
bq. > optional uint64 scannerId = 1;
bq. > optional Scan scan = 2;
bq. > optional uint32 numberOfRows = 3;
bq. > optional bool closeScanner = 4;
bq. > optional uint32 ttl = 5;
bq. > }
bq. >
bq. > message ScanResponse {
bq. > repeated Result result = 1;
bq. > optional uint64 scannerId = 2;
bq. > optional bool moreResults = 3;
bq. > optional uint32 ttl = 4;
bq. > }
bq. >
So we would do away with openScanner, next, and close, just do scan? Inside in the ScanRequest, we'd carry over the Scan specification each time? We'd be able to honor the current openScanner, next, close client-facing API but could add a new scan method to the public api that allowed passing the above specifications? Sounds good.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5575
-----------------------------------------------------------
On 2012-03-02 18:54:29, 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-02 18:54:29)
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 bb518b1
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.
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13220213#comment-13220213 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5503
-----------------------------------------------------------
pom.xml
<https://reviews.apache.org/r/4054/#comment11924>
I don't see this script in this patch
- Michael
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13218726#comment-13218726 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5415
-----------------------------------------------------------
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11804>
Why is this called 'ColumnFamilyMapProto' (as opposed to just, say, 'ColumnProto' or 'ColumnNameProto')?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11805>
What is this 'value' field? What is its serialization format?
- Shaneal
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221231#comment-13221231 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-03-02 20:07:30, Gregory Chanan wrote:
bq. > src/main/proto/hbase.proto, line 23
bq. > <https://reviews.apache.org/r/4054/diff/2/?file=87680#file87680line23>
bq. >
bq. > You don't have "option optimize_for = SPEED;" here.
will add.
bq. On 2012-03-02 20:07:30, Gregory Chanan wrote:
bq. > src/main/proto/RegionAdmin.proto, line 22
bq. > <https://reviews.apache.org/r/4054/diff/2/?file=87678#file87678line22>
bq. >
bq. > If we are getting rid of "Proto" in the message names, might as well get rid of it here too.
This one, I am not sure?
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5583
-----------------------------------------------------------
On 2012-03-02 18:54:29, 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-02 18:54:29)
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 bb518b1
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.
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Zhihong Yu (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217419#comment-13217419 ]
Zhihong Yu commented on HBASE-5443:
-----------------------------------
I think we should keep supporting multi(MultiAction) - asynchbase uses it heavily.
Take a look at src/MultiAction.java in asynchbase
> 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
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13220314#comment-13220314 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-03-01 19:35:08, Michael Stack wrote:
bq. > pom.xml, line 746
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86002#file86002line746>
bq. >
bq. > So, all proto generated files go to proto/generated? All into the same package? Thanks Jimmy.
bq. >
bq. > Also, mind checking out Deveraj's patch? I'd suggest at least reviewing it to figure if you fellas are using same conventions going all proto.
bq. >
bq. > Good stuff
I will change mine to protobuf/generated. Thrift is using protobuf.
I posted a comment on Deveraj's patch to ask if he can use sub-folder "generated".
Thanks a lot for review. I updated my patch per these comments.
I will post a different review once I am ready.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5511
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221151#comment-13221151 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/
-----------------------------------------------------------
(Updated 2012-03-02 18:54:29.710858)
Review request for hbase.
Summary
-------
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
Please review. I'd like to move ahead after we get to some agreement.
This addresses bug HBASE-5443.
https://issues.apache.org/jira/browse/HBASE-5443
Diffs
-----
pom.xml bb518b1
src/main/proto/RegionAdmin.proto PRE-CREATION
src/main/proto/RegionClient.proto PRE-CREATION
src/main/proto/hbase.proto PRE-CREATION
Diff: https://reviews.apache.org/r/4054/diff
Testing
-------
Thanks,
Jimmy
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13259890#comment-13259890 ]
Hudson commented on HBASE-5443:
-------------------------------
Integrated in HBase-TRUNK #2799 (See [https://builds.apache.org/job/HBase-TRUNK/2799/])
HBASE-5443 Convert admin protocol of HRegionInterface to PB (Revision 1329358)
Result = SUCCESS
stack :
Files :
* /hbase/trunk/security/src/main/java/org/apache/hadoop/hbase/ipc/SecureRpcEngine.java
* /hbase/trunk/security/src/main/java/org/apache/hadoop/hbase/ipc/SecureServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/AdminProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ClientProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/RpcEngine.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/AdminProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ClientProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/HBaseFsckRepair.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/SortedCopyOnWriteSet.java
* /hbase/trunk/src/main/protobuf/Admin.proto
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestHTableUtil.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFilesSplitRecovery.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13243036#comment-13243036 ]
Hudson commented on HBASE-5443:
-------------------------------
Integrated in HBase-TRUNK-security #155 (See [https://builds.apache.org/job/HBase-TRUNK-security/155/])
HBASE-5443 Create PB protocols for HRegionInterface (Revision 1307625)
Result = SUCCESS
stack :
Files :
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/HRegionPartitioner.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionAdminProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/package.html
* /hbase/trunk/src/main/protobuf
* /hbase/trunk/src/main/protobuf/README.txt
* /hbase/trunk/src/main/protobuf/RegionAdmin.proto
* /hbase/trunk/src/main/protobuf/RegionClient.proto
* /hbase/trunk/src/main/protobuf/hbase.proto
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Resolved] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Jimmy Xiang (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jimmy Xiang resolved HBASE-5443.
--------------------------------
Resolution: Fixed
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Updated] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Jimmy Xiang (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jimmy Xiang updated HBASE-5443:
-------------------------------
Issue Type: Task (was: Sub-task)
Parent: (was: HBASE-5305)
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13235780#comment-13235780 ]
Jimmy Xiang commented on HBASE-5443:
------------------------------------
I have done some code changes, and some tests failed. It is very hard to look into them. So I'd like to break it into small pieces and tag them one by one.
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221191#comment-13221191 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5575
-----------------------------------------------------------
This is another option for scan. This way, we will have only one scan method, no need to open/next/close.
Which one do you prefer? In the ScanRequest, either scannerId or scan must be specified, not both.
message Scan {
required RegionSpecifier region = 1;
repeated Column column = 2;
repeated Attribute attribute = 3;
optional bytes startRow = 4;
optional bytes stopRow = 5;
optional string filterName = 6;
optional TimeRange timeRange = 7;
optional uint32 maxVersions = 8 [default = 1];
optional bool cacheBlocks = 9 [default = true];
optional uint32 rowsToCache = 10;
optional uint32 batchSize = 11;
}
message ScanRequest {
optional uint64 scannerId = 1;
optional Scan scan = 2;
optional uint32 numberOfRows = 3;
optional bool closeScanner = 4;
optional uint32 ttl = 5;
}
message ScanResponse {
repeated Result result = 1;
optional uint64 scannerId = 2;
optional bool moreResults = 3;
optional uint32 ttl = 4;
}
- Jimmy
On 2012-03-02 18:54:29, 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-02 18:54:29)
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 bb518b1
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.
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253767#comment-13253767 ]
Hudson commented on HBASE-5443:
-------------------------------
Integrated in HBase-TRUNK #2755 (See [https://builds.apache.org/job/HBase-TRUNK/2755/])
HBASE-5443 Convert the client protocol of HRegionInterface to PB (Revision 1325937)
Result = FAILURE
stack :
Files :
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ClientScanner.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ScannerCallable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/filter/ParseConstants.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/HbaseObjectWritable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/AdminProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ClientProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/ClientProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/HBaseProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionAdminProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionClientProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Leases.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java
* /hbase/trunk/src/main/protobuf/Admin.proto
* /hbase/trunk/src/main/protobuf/Client.proto
* /hbase/trunk/src/main/protobuf/RegionAdmin.proto
* /hbase/trunk/src/main/protobuf/RegionClient.proto
* /hbase/trunk/src/main/protobuf/hbase.proto
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/io/TestHbaseObjectWritable.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/OOMERegionServer.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.java
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Zhihong Yu (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217459#comment-13217459 ]
Zhihong Yu commented on HBASE-5443:
-----------------------------------
That would break one RPC call into multiple RPC calls, right ?
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "binlijin (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13243949#comment-13243949 ]
binlijin commented on HBASE-5443:
---------------------------------
Hi guys,i have some question, why choose pb? why not avro or thrift?
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13220283#comment-13220283 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5511
-----------------------------------------------------------
pom.xml
<https://reviews.apache.org/r/4054/#comment11930>
So, all proto generated files go to proto/generated? All into the same package? Thanks Jimmy.
Also, mind checking out Deveraj's patch? I'd suggest at least reviewing it to figure if you fellas are using same conventions going all proto.
Good stuff
- Michael
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13218699#comment-13218699 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5416
-----------------------------------------------------------
pom.xml
<https://reviews.apache.org/r/4054/#comment11806>
does it make sense to assume protoc is in path?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11807>
HRI does have the tablename. Correct.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11808>
if the region is
foo,bar,1234.4567.
Then the encoded name is 4567? no?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11809>
Why would you give a regionname, as opposed to a tablename?
- Alex
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13218701#comment-13218701 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5421
-----------------------------------------------------------
pom.xml
<https://reviews.apache.org/r/4054/#comment11821>
It's not too bad - a bunch of hadoop-common stuff already does. (e.g. https://github.com/apache/hadoop-common/blob/branch-0.23/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/pom.xml)
- Shaneal
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13218700#comment-13218700 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-02-28 20:44:26, Michael Stack wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 118
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line118>
bq. >
bq. > Where does this come from in current API?
bq.
bq. Jimmy Xiang wrote:
bq. Yes.
It is from the current API. It seems nobody is using it. I will get rid of it.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5407
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217713#comment-13217713 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-02-27 23:52:25, 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. > oh, right... here's another place we could use HRegionSpecifier or something, then?
compactRegion/closeRegion/openRegion/splitRegion/flushRegion all can use it, right?
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5372
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217663#comment-13217663 ]
jiraposter@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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217393#comment-13217393 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/
-----------------------------------------------------------
Review request for hbase.
Summary
-------
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
Please review. I'd like to move ahead after we get to some agreement.
This addresses bug HBASE-5443.
https://issues.apache.org/jira/browse/HBASE-5443
Diffs
-----
pom.xml 066c027
src/main/proto/HRegionProtocol.proto PRE-CREATION
src/main/proto/hbase.proto PRE-CREATION
Diff: https://reviews.apache.org/r/4054/diff
Testing
-------
Thanks,
Jimmy
> 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
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13260260#comment-13260260 ]
Hudson commented on HBASE-5443:
-------------------------------
Integrated in HBase-TRUNK-security #182 (See [https://builds.apache.org/job/HBase-TRUNK-security/182/])
HBASE-5443 Convert admin protocol of HRegionInterface to PB (Revision 1329358)
Result = SUCCESS
stack :
Files :
* /hbase/trunk/security/src/main/java/org/apache/hadoop/hbase/ipc/SecureRpcEngine.java
* /hbase/trunk/security/src/main/java/org/apache/hadoop/hbase/ipc/SecureServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/AdminProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ClientProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnection.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HTable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/ServerCallable.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/ExecRPCInvoker.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/Invocation.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/RpcEngine.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/AdminProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ClientProtocol.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/ResponseConverter.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/protobuf/generated/AdminProtos.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionThriftServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServer.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/HBaseFsckRepair.java
* /hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/SortedCopyOnWriteSet.java
* /hbase/trunk/src/main/protobuf/Admin.proto
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/HConnectionTestingUtility.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestAdmin.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/client/TestHTableUtil.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/mapreduce/TestLoadIncrementalHFilesSplitRecovery.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegionServerBulkLoad.java
* /hbase/trunk/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
> Add PB-based calls to HRegionInterface
> --------------------------------------
>
> Key: HBASE-5443
> URL: https://issues.apache.org/jira/browse/HBASE-5443
> Project: HBase
> Issue Type: 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13218698#comment-13218698 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-02-28 23:28:19, Shaneal Manek wrote:
bq. > pom.xml, line 761
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86002#file86002line761>
bq. >
bq. > It's not too bad - a bunch of hadoop-common stuff already does. (e.g. https://github.com/apache/hadoop-common/blob/branch-0.23/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/pom.xml)
You are right. I copied over from there.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5421
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Jimmy Xiang (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221155#comment-13221155 ]
Jimmy Xiang commented on HBASE-5443:
------------------------------------
I updated the review with new diff, which incorporated the feedbacks from all reviewers. Thanks a lot for review.
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13218714#comment-13218714 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-02-28 23:08:15, Alex Newman wrote:
bq. > pom.xml, line 761
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86002#file86002line761>
bq. >
bq. > does it make sense to assume protoc is in path?
Hadoop requires that. I think HBase can do the same thing too.
bq. On 2012-02-28 23:08:15, Alex Newman wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 265
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line265>
bq. >
bq. > Why would you give a regionname, as opposed to a tablename?
The current HRegionInterface uses regionName. It is to lock a row in a region. Otherwise, we need to find which region is the row in.
bq. On 2012-02-28 23:08:15, Alex Newman wrote:
bq. > src/main/proto/HRegionProtocol.proto, line 174
bq. > <https://reviews.apache.org/r/4054/diff/1/?file=86003#file86003line174>
bq. >
bq. > if the region is
bq. >
bq. > foo,bar,1234.4567.
bq. >
bq. > Then the encoded name is 4567? no?
That's right. The encoded name is the hash.
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5416
-----------------------------------------------------------
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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221181#comment-13221181 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5573
-----------------------------------------------------------
This is a lot better already. One thing this doesn't address that I should've mentioned in my previous review is that the requests and responses still have a lot of duplicate data. For example if I "Get" a row that contains 3 KeyValue, in the response, on the wire, I'll get 3 times the key and 3 times the family.
pom.xml
<https://reviews.apache.org/r/4054/#comment12066>
You didn't take into account my comments on fixing this shell scripting from the previous iteration.
src/main/proto/RegionClient.proto
<https://reviews.apache.org/r/4054/#comment12069>
So a Get request can only fetch multiple Get from a single Region? That's not good. We need true multi-get, where you can fetch things from multiple regions on the same RegionServer at once.
src/main/proto/RegionClient.proto
<https://reviews.apache.org/r/4054/#comment12068>
trailing whitespaces
src/main/proto/RegionClient.proto
<https://reviews.apache.org/r/4054/#comment12070>
I don't know if we should let the client specify the TTL. Right now in HBase the TTL is hardcoded in the Configuration object of the RegionServer.
Actually I'm fine with allowing clients specify their own TTL as long as we bound the TTL with the servers' Configuration.
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment12071>
I still don't understand how these can be optional.
- Benoit
On 2012-03-02 18:54:29, 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-02 18:54:29)
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 bb518b1
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.
> 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
[jira] [Updated] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "Lars Hofhansl (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Lars Hofhansl updated HBASE-5443:
---------------------------------
Fix Version/s: 0.96.0
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13221221#comment-13221221 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
bq. On 2012-03-02 19:16:55, Benoit Sigoure wrote:
bq. > src/main/proto/RegionClient.proto, line 112
bq. > <https://reviews.apache.org/r/4054/diff/2/?file=87679#file87679line112>
bq. >
bq. > trailing whitespaces
Will remove it.
bq. On 2012-03-02 19:16:55, Benoit Sigoure wrote:
bq. > pom.xml, line 764
bq. > <https://reviews.apache.org/r/4054/diff/2/?file=87677#file87677line764>
bq. >
bq. > You didn't take into account my comments on fixing this shell scripting from the previous iteration.
Sorry. I forgot it. Good idea. I will address it in the next diff.
bq. On 2012-03-02 19:16:55, Benoit Sigoure wrote:
bq. > src/main/proto/hbase.proto, line 64
bq. > <https://reviews.apache.org/r/4054/diff/2/?file=87680#file87680line64>
bq. >
bq. > I still don't understand how these can be optional.
Ok, I can make family and qualifier required.
bq. On 2012-03-02 19:16:55, Benoit Sigoure wrote:
bq. > src/main/proto/RegionClient.proto, line 147
bq. > <https://reviews.apache.org/r/4054/diff/2/?file=87679#file87679line147>
bq. >
bq. > I don't know if we should let the client specify the TTL. Right now in HBase the TTL is hardcoded in the Configuration object of the RegionServer.
bq. >
bq. > Actually I'm fine with allowing clients specify their own TTL as long as we bound the TTL with the servers' Configuration.
I see. I will remove it. We can add it back if server supports it later on.
How about lockRow? Does the server support client specified TTL?
bq. On 2012-03-02 19:16:55, Benoit Sigoure wrote:
bq. > src/main/proto/RegionClient.proto, line 63
bq. > <https://reviews.apache.org/r/4054/diff/2/?file=87679#file87679line63>
bq. >
bq. > So a Get request can only fetch multiple Get from a single Region? That's not good. We need true multi-get, where you can fetch things from multiple regions on the same RegionServer at once.
I can move the region to Get. That means each Get need to specify a region, so it can be duplicated. Another option is to add another message like GetGroup which has a region and a set of Gets.
One more option is to make region optional in the request, and add an optional region to Get. The region in GetRequest will be used if there is no region specified in Get.
What do you think?
- Jimmy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5573
-----------------------------------------------------------
On 2012-03-02 18:54:29, 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-02 18:54:29)
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 bb518b1
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.
> 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
[jira] [Commented] (HBASE-5443) Add PB-based calls to
HRegionInterface
Posted by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-5443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13220823#comment-13220823 ]
jiraposter@reviews.apache.org commented on HBASE-5443:
------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review5552
-----------------------------------------------------------
This seems to be close to a one-to-one mapping with the current interface today. I don't know if this is the intent or whether you're willing to completely redesign the look of the API too. Maybe it's to ease the transition?
I'd like to see a request type to do one-shot scans. Something where you don't even get a scanner ID. You pass parameters like to open a scanner, you say up to how many rows or bytes you want to retrieve, and you get just that in one shot.
When opening a actual scanner, we also need to be able to get the first batch of scan results at the same time we open the scanner. This is a must-have IMO. And we need to be able to request to close the scanner while fetching a batch of results.
It would be nice to have a "keep-alive" request for existing scanners. Something to tell the server "I'm not fetching anything from this scanner right now, but please keep it open by reseting its TTL, don't close it just because I haven't used it for a while".
Please, please, please, consider shortening the name of all these protobufs and dropping the Proto suffix. The current names are unnecessarily long or aren't intuitive (e.g. "columnFamily" for something that describes the multiple things you're trying to get out of a row) or are too redundant (e.g. "KeyType keyType").
Regarding the lack of "multi" RPC, I think this is a good thing. "multi" is a big mess that was only marginally better than its horrible "multiPut" predecessor. This proposal already supports multi-everything, it just doesn't support batching different kind of operations in the same RPC, which isn't a big deal IMO.
pom.xml
<https://reviews.apache.org/r/4054/#comment11997>
Do this instead:
if which cygpath >/dev/null 2>/dev/null; then
# Windows
else
# Not Windows
fi
pom.xml
<https://reviews.apache.org/r/4054/#comment11998>
Simply do:
if $IS_WIN; then
pom.xml
<https://reviews.apache.org/r/4054/#comment12016>
Actually you can just remove the whole $IS_WIN business and everything. Simply fix PROTO_DIR and JAVA_DIR when on Windows before calling protoc.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment11999>
I find the Proto suffix unnecessary and long. If you truly want a suffix, PB would be shorter, but no suffix would be better IMO.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment12004>
Use "option optimize_for = SPEED", it makes a big difference.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment12006>
I'd call this just "Columns".
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment12005>
I would recommend to pluralize all "repeated" fields. This will make for nicer code where you'll be able to write something along the lines of:
for (byte[] qualifier : pb.qualifiers())
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment12010>
The thing that append() and increment() have in common is that they're atomic operations that don't require a read-modify-write from the client. So maybe AtomicOp would be better?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment12012>
Just call this Columns.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment12019>
What's the meaning of this? How do we know what has been processed and what hasn't?
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment12000>
I'd vote for adding this right now. It's easy to add directly and would be a huge improvement for short scans (which are super common).
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment12007>
We need to have a way to get feedback from the server about the TTL of the scanner. How long can the client hold on to the scanner before the server will kill it. Add a field here so that the server can communicate the TTL to the client.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment12001>
Please add an "optional boolean close" to request that the scanner be closed after returning this batch of results. This can help clients eliminate the "CloseScannerRequestProto" when they know they're going to close the scanner after this batch.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment12017>
This name is inconsistent with the name of the request. By your scheme it should be named "LockRowResponseProto" – although I'd much prefer "LockResp" or something like that.
src/main/proto/HRegionProtocol.proto
<https://reviews.apache.org/r/4054/#comment12018>
This needs to have a field specifying how long the server is willing to hand out the lock for.
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment12015>
Call these fields just "from" and "to".
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment12002>
Why are all these fields optional? How can a KeyValue not have a family or a qualifier or a timestamp?
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment12003>
I'd recommend naming this "timestamp" not "time".
src/main/proto/hbase.proto
<https://reviews.apache.org/r/4054/#comment12013>
Just call this "type".
- Benoit
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