subject:"\[jira\] \[Commented\] \(HBASE\-5619\) Create PB protocols for HRegionInterface"

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-30 Thread Jimmy Xiang (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13242720#comment-13242720
 ] 

Jimmy Xiang commented on HBASE-5619:


@Stack, thanks!

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: 5619v6.txt, 5619v6.txt, hbase-5619.patch, 
 hbase-5619_v3.patch, hbase-5619_v4.patch, hbase-5619_v5.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-30 Thread Hadoop QA (Commented) (JIRA)

[
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13242764#comment-13242764
]

Hadoop QA commented on HBASE-5619:
--

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12520665/5619v6.txt
against trunk revision .

+1 @author. The patch does not contain any @author tags.

-1 tests included. The patch doesn't appear to include any new or modified
tests.
Please justify why no new tests are needed for this
patch.
Also please list what manual steps were performed to
verify this patch.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac
compiler warnings.

-1 findbugs. The patch appears to introduce 70 new Findbugs (version
1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of
release audit warnings.

-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.client.TestFromClientSide
org.apache.hadoop.hbase.mapreduce.TestImportTsv
org.apache.hadoop.hbase.mapred.TestTableMapReduce
org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat

Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/1354//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/1354//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/1354//console

This message is automatically generated.

Create PB protocols for HRegionInterface

Key: HBASE-5619
URL: https://issues.apache.org/jira/browse/HBASE-5619
Project: HBase
Issue Type: Sub-task
Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
Fix For: 0.96.0

Attachments: 5619v6.txt, 5619v6.txt, hbase-5619.patch,
hbase-5619_v3.patch, hbase-5619_v4.patch, hbase-5619_v5.patch

Subtask of HBase-5443, separate HRegionInterface into admin protocol and
client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-30 Thread stack (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13242794#comment-13242794
 ] 

stack commented on HBASE-5619:
--

I tried TestFromClientSide a few times locally and its passing fine.

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: 5619v6.txt, 5619v6.txt, hbase-5619.patch, 
 hbase-5619_v3.patch, hbase-5619_v4.patch, hbase-5619_v5.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread Jimmy Xiang (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13241374#comment-13241374
 ] 

Jimmy Xiang commented on HBASE-5619:


@Stack, could you please commit this patch?  I do have some changes to pb 
files. But I'd like to address them in HBASE-5620.

Thanks.


 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread stack (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13241560#comment-13241560
 ] 

stack commented on HBASE-5619:
--

Can you fix this Jimmy?  Happens when I try to build after applying the patch:

{code}
main:
 [exec] target/compile-proto.sh: line 12: protoc: command not found
 [exec] target/compile-proto.sh: line 12: protoc: command not found
 [exec] target/compile-proto.sh: line 12: protoc: command not found
[INFO] 
[ERROR] BUILD ERROR
[INFO] 
[INFO] An Ant BuildException has occured: exec returned: 127
{code}

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread Zhihong Yu (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13241594#comment-13241594
 ] 

Zhihong Yu commented on HBASE-5619:
---

@Stack:
Was protoc installed on the box you performed the build.

I did a clean build after applying patch v4 and didn't see the above error.

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread Jimmy Xiang (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13241622#comment-13241622
 ] 

Jimmy Xiang commented on HBASE-5619:


@Stack, could you please install protoc and give it a try again?
From now on, we need protoc to compile. :)

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread stack (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13241756#comment-13241756
 ] 

stack commented on HBASE-5619:
--

@Jimmy Is there a maven plugin for protoc so folks don't have to install it?  
At least, we should be checking for its presence and complaining w/ a pointer 
to a how-to-install doc if not present?  Is that possible?

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread Jimmy Xiang (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13241819#comment-13241819
 ] 

Jimmy Xiang commented on HBASE-5619:


@Stack, so far, I could not find a good protoc maven plugin. I don't remember I 
tried to install it on my Ubuntu.
That's the download site for protobuf compiler: 
http://code.google.com/p/protobuf/downloads/list
But for Linux, I think it is easy to install with rpm/apt-get.

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread Enis Soztutar (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13241863#comment-13241863
 ] 

Enis Soztutar commented on HBASE-5619:
--

I think for compiling protoc installation should not be a dependency. Why don't 
we add the generated java files to the source tree, so that only if you change 
the protoc definitions, you would have to recompile the .proto files. As far as 
I know, this is how it is done for thrift dependencies in other projects. 

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread Jimmy Xiang (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13241880#comment-13241880
 ] 

Jimmy Xiang commented on HBASE-5619:


But for proto files, other projects depend on protoc, for example HADOOP/HDFS. 
We are moving towards pb, protoc dependency should be fine.
I can try to setup a temp protoc dynamically.

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread Enis Soztutar (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13241916#comment-13241916
 ] 

Enis Soztutar commented on HBASE-5619:
--

I was suggesting adding the generated files under src/, and adding the runtime 
protobuf dependency in maven. But if it won't work, we can go with requiring 
protoc. Dynamic installation will be a little bit overkill. 

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread stack (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13241937#comment-13241937
 ] 

stack commented on HBASE-5619:
--

I like the idea of checking in the generated protobufs files.  They'll change 
rarely.  Let the fella who generates them have to worry about his protoc setup 
(This is what we do now for thrift and avro?)

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread Jimmy Xiang (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13241954#comment-13241954
 ] 

Jimmy Xiang commented on HBASE-5619:


That's what do for thrift now, not avro.

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-29 Thread Hadoop QA (Commented) (JIRA)

[
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13242040#comment-13242040
]

Hadoop QA commented on HBASE-5619:
--

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12520550/hbase-5619_v5.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac
compiler warnings.

-1 findbugs. The patch appears to introduce 68 new Findbugs (version
1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of
release audit warnings.

-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.mapreduce.TestImportTsv

Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/1350//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/1350//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/1350//console

This message is automatically generated.

Create PB protocols for HRegionInterface

Attachments: hbase-5619.patch, hbase-5619_v3.patch,
hbase-5619_v4.patch, hbase-5619_v5.patch

Subtask of HBase-5443, separate HRegionInterface into admin protocol and
client protocol, create the PB protocol buffer files

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-27 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

[
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13239205#comment-13239205
]

jirapos...@reviews.apache.org commented on HBASE-5619:
--

bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. src/main/proto/RegionAdmin.proto, line 21
bq. https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line21
bq.
bq. What did we figure on the package name? Shouldn't it agree w/ the
dir that holds the .proto files at src/main? Currently one is protobuf and the
other is proto.
bq.
bq. Jimmy Xiang wrote:
bq. There is already an exiting folder called protobuf (rest). Let me
change the dir holds the .proto files under src/main to protobuf.

good

Hmm. Moving out these flags changes the current 'model' but in a direction we
should be headed in. The split/offline stuff were stuffed into HRI in the
first place just because this was an easy way to pass these transient states
around in hbase; they also are less important now in HRI though still depended
on when we scan meta IIRC. Its probably better to evolve toward an HRI that is
immutable once made. So I'd be down w/ moving them out but its up to you. It
might be easier on you achieving parity w/ first commit of pb work if the pb
classes are like the internals they will be feeding into.

bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. src/main/proto/RegionAdmin.proto, line 70
bq. https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line70
bq.
bq. Should we be repeating the API documentation that is up in the
HRegionInterface that this .proto replaces here? If its not here, where will
it be? Not all of the javadoc should make it over -- the stuff that says
nothing shouldn't but some is of worth. What you think?
bq.
bq. Jimmy Xiang wrote:
bq. I though about this. I added documentation to RegionClient.proto.
For methods in RegionAdmin, the method names seem to be very clear. I will
take a look again and document where confusing/misunderstand could arise.

I'd be fine w/ the doc being sparse but there are a few cases where doc is
necessary; e.g. the one I cite above.

Does that imply a synchronous call? I thought flushCache just queued the flush
then returned to the client w/o actually waiting on flush to complete.

bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. src/main/proto/RegionClient.proto, line 52
bq. https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line52
bq.
bq. This is new? Being able to do this? How will it be used?
bq.
bq. Jimmy Xiang wrote:
bq. Some call expects a region name, some call expects an encoded region
name. With a specifier, we can handle both.
bq. Encoded region name works only if the region is on-line. If we can't
find the region based on the specifier, a region not found exception will be
thrown.
bq.
bq. So we can simplify the request a little bit since we don't have to ask
for region name, and encoded region name, and check only if one is specified.
bq.
bq. I will add a comment for it.

bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. src/main/proto/RegionClient.proto, line 66
bq. https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line66
bq.
bq. This is new feature on get? Or just special handling of an
attribute?
bq.
bq. Jimmy Xiang wrote:
bq. This is for the exist() call. In this case, the caller doesn't care
about the result. They just want to know if the row is there. It is not
special handling of an attribute.

The current implementation actually does the fetch and in the client checks it
null or not IIRC? Or is it all serverside? So you add this to the Get so you
don't have

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-27 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13239733#comment-13239733
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--



bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 338
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line338
bq.  
bq.   Does this belong here?  You provide it as a param when making the 
request.
bq.  
bq.  Jimmy Xiang wrote:
bq.  Yes, this is for Region protocol, not client API.  You need to specify 
for which region this action will be executed on, right?
bq.  
bq.  Michael Stack wrote:
bq.  Yes.  I got into habit of complaining about the region specs. They 
make sense when they are params.  Its regions internal to Get, etc., that 
causes me difficulty.

I see.  I will move it out to the request so it is easier to understand.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 233
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line233
bq.  
bq.   Why this when regions can change.  Also,  a Scan can traverse many 
regions so what is the regionspecifier referring to?  The first?
bq.  
bq.  Jimmy Xiang wrote:
bq.  Yes, it is the first region.
bq.  
bq.  Michael Stack wrote:
bq.  You know what I am going to say (smile)

Right. Shall I rename it to startRegion, or move it out, or just get rid of it 
and figure out the region dynamically based on the startRow?


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 193
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line193
bq.  
bq.   What is this implementing from current Delete?  The delete family?
bq.   
bq.   This is returned to the client?
bq.   
bq.   How do we do a column that has no qualifier?  Thats possible in 
hbase.
bq.  
bq.  Jimmy Xiang wrote:
bq.  To delete a family, you don't specify the qualifier. To delete a only 
one version of a column, you sepcify the family, qualifer, timestamp, and set 
oneVersion to true.
bq.  To delete all version of a column, you specify the family, qualifier
bq.  
bq.  What do you mean a column that has no qualifier? Are you referring to 
Delete.deleteColumns(family, qualifier, timestamp), and qualifer = null?
bq.  
bq.  Michael Stack wrote:
bq.  Yeah.  You can put a value at row, family, null qualifier, ts. You can 
also delete it (might not be a good idea but you can do it).

In this case, all columns in the family for this row will be deleted, right?


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 91
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line91
bq.  
bq.   I thought we could set this into the Get above?  Why have it here as 
separate param?
bq.  
bq.  Jimmy Xiang wrote:
bq.  This is used as a default region.  If you have multiple Gets on the 
same region, you don't have to specify it every where.
bq.  
bq.  Michael Stack wrote:
bq.  Could we just have it out here as a param on the get and not in the 
Get object?  Will that work (I think I understand why Result needs it internal 
to cut down on objects we need to support multiresponses)

Sure, I can do that for get.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 84
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line84
bq.  
bq.   Why is the Get polluted by multiGet stuff?
bq.  
bq.  Jimmy Xiang wrote:
bq.  The interface is kind of similar, but very flexible. Any problem with 
that?
bq.  
bq.  Michael Stack wrote:
bq.  On 'Any problem with that?', no.  Just trying to avoid redundancy that 
makes the Interface imprecise and therefore hard to grok.

I see.  I will remove the region from Get. It is ok not to support multiGet, 
right?


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 82
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line82
bq.  
bq.   Why we need to add a region to the Get even optionally?
bq.  
bq.  Jimmy Xiang wrote:
bq.  That's to support multiple Get requests in one call.  Each Get request 
can have its own region.  If all Gets are for one region, the caller can 
specify the region in the request level.
bq.  
bq.  Michael Stack wrote:
bq.  I see specifying the region name in this GetRequest but not sure about 
why we'd have region in the Get object itself if we are doing it here on the 
invocation.  Seems like the two contend?

I will remove it from Get object to avoid confusion for now.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 71
bq.

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-27 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

[
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13239783#comment-13239783
]

jirapos...@reviews.apache.org commented on HBASE-5619:
--

bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. src/main/proto/RegionAdmin.proto, line 87
bq. https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line87
bq.
bq. Isn't the response currently a void? And isnt' flush async (IIRC).
If so, under what circumstance would we be able to fill out this response?
bq.
bq. Jimmy Xiang wrote:
bq. I was thinking to set it if the HRegion.flushCache call returned true.
It is just informational.
bq.
bq. Michael Stack wrote:
bq. Does that imply a synchronous call? I thought flushCache just queued
the flush then returned to the client w/o actually waiting on flush to complete.

I checked the code and it seems that flushCache is a synchronous call. Should
we make it async? If so, I can make flushed to queued.

Sure, I will add more doc where confusion could happen.

bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. src/main/proto/RegionAdmin.proto, line 35
bq. https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line35
bq.
bq. What are these two booleans broken out? Aren't they in they
attributes of HRI already? Why repeat them?
bq.
bq. Jimmy Xiang wrote:
bq. I used to put these transient parameters in the protobuff RegionInfo
as well. However Todd thought it's better to put them outside.
bq.
bq. What'd do you think? To me, it is fine either way. However, if we
are going to replace HRI with the protobuff later on, it may be better to put
them together.
bq.
bq. Michael Stack wrote:
bq. Hmm. Moving out these flags changes the current 'model' but in a
direction we should be headed in. The split/offline stuff were stuffed into
HRI in the first place just because this was an easy way to pass these
transient states around in hbase; they also are less important now in HRI
though still depended on when we scan meta IIRC. Its probably better to evolve
toward an HRI that is immutable once made. So I'd be down w/ moving them out
but its up to you. It might be easier on you achieving parity w/ first commit
of pb work if the pb classes are like the internals they will be feeding into.

Ok, I can move them back. We can get back to this when time comes.

bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. src/main/proto/RegionClient.proto, line 77
bq. https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line77
bq.
bq. what is processed?
bq.
bq. Jimmy Xiang wrote:
bq. Sometimes, for some action, for example delete, they don't care about
the result. They just want to know if the action is processed or not. This is
for this kind use case.
bq.
bq. Michael Stack wrote:
bq. Needs comment

Will add comment

bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. src/main/proto/RegionClient.proto, line 159
bq. https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line159
bq.
bq. Again, does it make sense having this in here? I mean regions come
and go -- e.g. split -- so you could have reference to non-existent region.
This stuff of tying a mutation to a particular region should be done external
to the mutations themselves?
bq.
bq. Is this trying to do multiaction? Maybe that should be a message
apart from mutate? The new message would have region name and the mutate?
bq.
bq. Jimmy Xiang wrote:
bq. Similar to Gets, I'd like to make the interface flexible. Sounds you
prefer to have simple interface instead, right? That means we may have to have
too many methods. For java interface, it is ok and preferred to have many
methods. But for RPC, I think it is better to have fewer and flexible methods.
bq.
bq. Michael Stack wrote:
bq. 'But for RPC, I think it is better to have fewer and flexible
methods.' Fair enough. Would like

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-27 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

[
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13239876#comment-13239876
]

jirapos...@reviews.apache.org commented on HBASE-5619:
--

bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. src/main/proto/RegionClient.proto, line 338
bq. https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line338
bq.
bq. Does this belong here? You provide it as a param when making the
request.
bq.
bq. Jimmy Xiang wrote:
bq. Yes, this is for Region protocol, not client API. You need to specify
for which region this action will be executed on, right?
bq.
bq. Michael Stack wrote:
bq. Yes. I got into habit of complaining about the region specs. They
make sense when they are params. Its regions internal to Get, etc., that
causes me difficulty.
bq.
bq. Jimmy Xiang wrote:
bq. I see. I will move it out to the request so it is easier to
understand.

I think this would be good.

bq. On 2012-03-26 23:21:58, Michael Stack wrote:
bq. src/main/proto/RegionClient.proto, line 233
bq. https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line233
bq.
bq. Why this when regions can change. Also, a Scan can traverse many
regions so what is the regionspecifier referring to? The first?
bq.
bq. Jimmy Xiang wrote:
bq. Yes, it is the first region.
bq.
bq. Michael Stack wrote:
bq. You know what I am going to say (smile)
bq.
bq. Jimmy Xiang wrote:
bq. Right. Shall I rename it to startRegion, or move it out, or just get
rid of it and figure out the region dynamically based on the startRow?
bq.

My guess is that this region in a Scan won't do you any good. The client
figures where the Scan is at any one time and what region it should be on.
There is probably no value having this in the Scan.

That might be easiest. I think yeah, they don't really belong in here and
we've made some work undoing their need to be i here but lets do that in
another issue

Then I was wrong in my supposition. Lets do what you suggest, an actual return
with a response w/ some value in it.

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-27 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13239935#comment-13239935
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--



bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 193
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line193
bq.  
bq.   What is this implementing from current Delete?  The delete family?
bq.   
bq.   This is returned to the client?
bq.   
bq.   How do we do a column that has no qualifier?  Thats possible in 
hbase.
bq.  
bq.  Jimmy Xiang wrote:
bq.  To delete a family, you don't specify the qualifier. To delete a only 
one version of a column, you sepcify the family, qualifer, timestamp, and set 
oneVersion to true.
bq.  To delete all version of a column, you specify the family, qualifier
bq.  
bq.  What do you mean a column that has no qualifier? Are you referring to 
Delete.deleteColumns(family, qualifier, timestamp), and qualifer = null?
bq.  
bq.  Michael Stack wrote:
bq.  Yeah.  You can put a value at row, family, null qualifier, ts. You can 
also delete it (might not be a good idea but you can do it).
bq.  
bq.  Jimmy Xiang wrote:
bq.  In this case, all columns in the family for this row will be deleted, 
right?
bq.  
bq.  Michael Stack wrote:
bq.  IIRC, yes.  See http://hbase.apache.org/book.html#delete where I tried 
to doc the options

I see.  Will cover this scenario.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 84
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line84
bq.  
bq.   Why is the Get polluted by multiGet stuff?
bq.  
bq.  Jimmy Xiang wrote:
bq.  The interface is kind of similar, but very flexible. Any problem with 
that?
bq.  
bq.  Michael Stack wrote:
bq.  On 'Any problem with that?', no.  Just trying to avoid redundancy that 
makes the Interface imprecise and therefore hard to grok.
bq.  
bq.  Jimmy Xiang wrote:
bq.  I see.  I will remove the region from Get. It is ok not to support 
multiGet, right?
bq.  
bq.  Michael Stack wrote:
bq.  We should support multiget, yes.  Do you have to make a new message to 
do that if you remove the regionspec from Get?  If so, maybe that is ok?  If 
not, i'm fine w/ it as optional regionspec in Get.  I think it would be fine to 
purge regionspec from the Get, Put, Delete, objects and then add messages to 
support multactions.  Up to you.  Just doc it and be consistent (smile).

Got it.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 214
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line214
bq.  
bq.   Yeah, why have regionspecified in the mutate if you are going to 
provide it as a param too?
bq.  
bq.  Jimmy Xiang wrote:
bq.  The same reason as for Get.  It is used as a default region in case 
all/most mutates are for a same region.
bq.  
bq.  Michael Stack wrote:
bq.  Having 'default' region doesn't sound right, even if you are trying to 
be flexible.
bq.  
bq.  Jimmy Xiang wrote:
bq.  I see. For get, we put the region in the request level.  I prefer to 
do the same for mutate.  But if we put the region in request for mutate, we 
have to have a several
bq.  call to handle RowMutations in the same call.  I can try to handle 
RowMutations in the multi call.
bq.  
bq.  Another thing is that, if we put the region in the request level, do 
we need to support multiple Get per get request?  How about multiple Mutate per 
mutate request?
bq.  
bq.  Michael Stack wrote:
bq.  On I can try to handle RowMutations in the multi call., good.   So, 
we won't do multiple calls to do many?

That's right. We don't do multiple calls to do many.  I will simplify the 
interface.


- Jimmy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review6365
---


On 2012-03-26 20:14:22, Jimmy Xiang wrote:
bq.  
bq.  ---
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/4054/
bq.  ---
bq.  
bq.  (Updated 2012-03-26 20:14:22)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  This is the first draft of the ProtoBuff HRegionProtocol.  The 
corresponding java vs pb method mapping is attached to the jira: 
https://issues.apache.org/jira/browse/HBASE-5443
bq.  
bq.  Please review.  I'd like to move ahead after we get to some agreement.
bq.  
bq.  
bq.  This addresses bug HBASE-5619.
bq.

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-27 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13239988#comment-13239988
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/
---

(Updated 2012-03-27 21:57:06.565352)


Review request for hbase.


Changes
---

Addressed Stack's comments.


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-5619.
https://issues.apache.org/jira/browse/HBASE-5619


Diffs (updated)
-

  pom.xml 10b13ef 
  src/main/protobuf/RegionAdmin.proto PRE-CREATION 
  src/main/protobuf/RegionClient.proto PRE-CREATION 
  src/main/protobuf/hbase.proto PRE-CREATION 

Diff: https://reviews.apache.org/r/4054/diff


Testing
---


Thanks,

Jimmy



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-27 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13240021#comment-13240021
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review6453
---

Ship it!


I'm game for committing this.  My guess is that it'll need some massage before 
we arrive at final for 0.96.  Will let this bake a day or so before making 
actual commit in case someone else wants to take a looksee.  Nice stuff Jimmy


src/main/protobuf/RegionClient.proto
https://reviews.apache.org/r/4054/#comment14078

This seems off.



src/main/protobuf/RegionClient.proto
https://reviews.apache.org/r/4054/#comment14079

Great



src/main/protobuf/RegionClient.proto
https://reviews.apache.org/r/4054/#comment14080

Great



src/main/protobuf/RegionClient.proto
https://reviews.apache.org/r/4054/#comment14081

oh, here is getclosestrowbefore.  good.  easy to remove.



src/main/protobuf/RegionClient.proto
https://reviews.apache.org/r/4054/#comment14082

Does this break if qualifier is null?  It should work?



src/main/protobuf/RegionClient.proto
https://reviews.apache.org/r/4054/#comment14083

This modeling seems a little odd.   I'd think that a Mutate would have a 
column and then a ts and a value but lets leave it this way.  My guess is that 
you've figured that this will make your life easier going forward (if so, lets 
keep this 'model'...)



src/main/protobuf/RegionClient.proto
https://reviews.apache.org/r/4054/#comment14085

So, can an open scanner return results?  Curently returns scanid only.  
This looks like it could return scanid AND first set of results.  That'd be 
cool.


- Michael


On 2012-03-27 21:57:06, 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-27 21:57:06)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  This is the first draft of the ProtoBuff HRegionProtocol.  The 
corresponding java vs pb method mapping is attached to the jira: 
https://issues.apache.org/jira/browse/HBASE-5443
bq.  
bq.  Please review.  I'd like to move ahead after we get to some agreement.
bq.  
bq.  
bq.  This addresses bug HBASE-5619.
bq.  https://issues.apache.org/jira/browse/HBASE-5619
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq.pom.xml 10b13ef 
bq.src/main/protobuf/RegionAdmin.proto PRE-CREATION 
bq.src/main/protobuf/RegionClient.proto PRE-CREATION 
bq.src/main/protobuf/hbase.proto PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/4054/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Jimmy
bq.  
bq.



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch, hbase-5619_v3.patch, 
 hbase-5619_v4.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-27 Thread Hadoop QA (Commented) (JIRA)

[
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13240027#comment-13240027
]

Hadoop QA commented on HBASE-5619:
--

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12520200/hbase-5619_v4.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac
compiler warnings.

-1 findbugs. The patch appears to introduce 73 new Findbugs (version
1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of
release audit warnings.

-1 core tests. The patch failed these unit tests:
org.apache.hadoop.hbase.replication.TestReplication
org.apache.hadoop.hbase.mapreduce.TestImportTsv
org.apache.hadoop.hbase.mapred.TestTableMapReduce
org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat

Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/1323//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/1323//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/1323//console

This message is automatically generated.

Create PB protocols for HRegionInterface

Attachments: hbase-5619.patch, hbase-5619_v3.patch,
hbase-5619_v4.patch

Subtask of HBase-5443, separate HRegionInterface into admin protocol and
client protocol, create the PB protocol buffer files

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-26 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13238699#comment-13238699
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review6354
---


I'm looking at differences between r1 and r4 and I don't see anything new in 
HRegionProtocol.proto ..?


pom.xml
https://reviews.apache.org/r/4054/#comment13781

Can you please kill all the trailing whitespaces while you're at it?



pom.xml
https://reviews.apache.org/r/4054/#comment13779

Just do if which cygpath 2/dev/null instead of doing it on 2 lines and 
testing $?



pom.xml
https://reviews.apache.org/r/4054/#comment13782

Just do for PROTO_FILE in $UNIX_PROTO_DIR/*.proto, I don't think the ls 
is really necessary here.



src/main/proto/HRegionProtocol.proto
https://reviews.apache.org/r/4054/#comment13783

I'm not seeing that you added it.


- Benoit


On 2012-03-23 19:29:52, 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-23 19:29:52)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  This is the first draft of the ProtoBuff HRegionProtocol.  The 
corresponding java vs pb method mapping is attached to the jira: 
https://issues.apache.org/jira/browse/HBASE-5443
bq.  
bq.  Please review.  I'd like to move ahead after we get to some agreement.
bq.  
bq.  
bq.  This addresses bug HBASE-5619.
bq.  https://issues.apache.org/jira/browse/HBASE-5619
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq.pom.xml 10b13ef 
bq.src/main/proto/RegionAdmin.proto PRE-CREATION 
bq.src/main/proto/RegionClient.proto PRE-CREATION 
bq.src/main/proto/hbase.proto PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/4054/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Jimmy
bq.  
bq.



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-26 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13238704#comment-13238704
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review6356
---


Oh my, it's just that review board sucks, it's still showing the file as being 
added when comparing r1 and r4, even though the file is no longer in r4.

- Benoit


On 2012-03-23 19:29:52, 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-23 19:29:52)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  This is the first draft of the ProtoBuff HRegionProtocol.  The 
corresponding java vs pb method mapping is attached to the jira: 
https://issues.apache.org/jira/browse/HBASE-5443
bq.  
bq.  Please review.  I'd like to move ahead after we get to some agreement.
bq.  
bq.  
bq.  This addresses bug HBASE-5619.
bq.  https://issues.apache.org/jira/browse/HBASE-5619
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq.pom.xml 10b13ef 
bq.src/main/proto/RegionAdmin.proto PRE-CREATION 
bq.src/main/proto/RegionClient.proto PRE-CREATION 
bq.src/main/proto/hbase.proto PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/4054/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Jimmy
bq.  
bq.



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-26 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13238766#comment-13238766
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--



bq.  On 2012-03-26 19:15:13, Benoit Sigoure wrote:
bq.   pom.xml, line 780
bq.   https://reviews.apache.org/r/4054/diff/1-4/?file=86002#file86002line780
bq.  
bq.   Just do if which cygpath 2/dev/null instead of doing it on 2 
lines and testing $?

Sure, will fix.


bq.  On 2012-03-26 19:15:13, Benoit Sigoure wrote:
bq.   pom.xml, line 787
bq.   https://reviews.apache.org/r/4054/diff/1-4/?file=86002#file86002line787
bq.  
bq.   Just do for PROTO_FILE in $UNIX_PROTO_DIR/*.proto, I don't think 
the ls is really necessary here.

You are right.  It works without ls.  Fixed.


bq.  On 2012-03-26 19:15:13, Benoit Sigoure wrote:
bq.   pom.xml, line 334
bq.   https://reviews.apache.org/r/4054/diff/1-4/?file=86002#file86002line334
bq.  
bq.   Can you please kill all the trailing whitespaces while you're at it?

Sure, I will do that.


- Jimmy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review6354
---


On 2012-03-23 19:29:52, 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-23 19:29:52)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  This is the first draft of the ProtoBuff HRegionProtocol.  The 
corresponding java vs pb method mapping is attached to the jira: 
https://issues.apache.org/jira/browse/HBASE-5443
bq.  
bq.  Please review.  I'd like to move ahead after we get to some agreement.
bq.  
bq.  
bq.  This addresses bug HBASE-5619.
bq.  https://issues.apache.org/jira/browse/HBASE-5619
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq.pom.xml 10b13ef 
bq.src/main/proto/RegionAdmin.proto PRE-CREATION 
bq.src/main/proto/RegionClient.proto PRE-CREATION 
bq.src/main/proto/hbase.proto PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/4054/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Jimmy
bq.  
bq.



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-26 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13238768#comment-13238768
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/
---

(Updated 2012-03-26 20:14:22.677676)


Review request for hbase.


Changes
---

Addressed Benoit's comments.


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-5619.
https://issues.apache.org/jira/browse/HBASE-5619


Diffs (updated)
-

  pom.xml 10b13ef 
  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



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-26 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

[
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13238972#comment-13238972
]

jirapos...@reviews.apache.org commented on HBASE-5619:
--

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review6365
---

Excellent.

It looks like we can commit this w/o breaking whats currently there?

src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13813

What did we figure on the package name? Shouldn't it agree w/ the dir that
holds the .proto files at src/main? Currently one is protobuf and the other is
proto.

src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13835

What are these two booleans broken out? Aren't they in they attributes of
HRI already? Why repeat them?

src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13836

Good. Its kinda dumb the way our HRegionInterface is now where it has an
override, one that takes single family and another that takes an array. Thanks
for collapsing.

src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13837

Should we be repeating the API documentation that is up in the
HRegionInterface that this .proto replaces here? If its not here, where will
it be? Not all of the javadoc should make it over -- the stuff that says
nothing shouldn't but some is of worth. What you think?

src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13838

Again, thanks for doing the work collapsing the overrides that are up in
HRegionInterface.

src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13839

Isn't the response currently a void? And isnt' flush async (IIRC). If
so, under what circumstance would we be able to fill out this response?

src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13842

WALKey maps to HLogKey? Maybe add a comment to this effect?

src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13846

Good. I like this method name better. Should be a comment which points
back to the old name? Or what you think?

src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13847

Yeah, this proto is missing commentary. I mean, the return here should be
explained?

src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13850

This will for sure grow w/ time.

src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13855

Nice. So you are splitting HRegionInterface into admin and client? Good.

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13862

This is new? Being able to do this? How will it be used?

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13863

This is new feature on get? Or just special handling of an attribute?

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13864

We don't have this in the java Result. I don't understand why this is
making its way into the object.

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13865

ditto

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13866

what is processed?

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13867

Why we need to add a region to the Get even optionally?

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13868

Why is the Get polluted by multiGet stuff?

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13869

I thought we could set this into the Get above? Why have it here as
separate param?

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13870

Good stuff

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13872

This is a new feature?

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13881

A type rather than have the mutation type specified as a subclass?

A mutate is a delete or put?

src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13882

Again, does it make sense having this in here? I mean regions come and go
-- e.g. split -- so you could have reference to non-existent region. This
stuff of tying a mutation to a particular region should be done external to the
mutations themselves?

Is this trying to do multiaction? Maybe that should be a message apart
from mutate? The new message would have region name and the mutate?

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-26 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13239140#comment-13239140
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--



bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   Excellent.
bq.   
bq.   It looks like we can commit this w/o breaking whats currently there?

That's right.  It won't break anything.  It is just proto files in this patch. 
Thanks for reviewing.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionAdmin.proto, line 21
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line21
bq.  
bq.   What did we figure on the package name?  Shouldn't it agree w/ the 
dir that holds the .proto files at src/main?  Currently one is protobuf and the 
other is proto.

There is already an exiting folder called protobuf (rest).  Let me change the 
dir holds the .proto files under src/main to protobuf.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionAdmin.proto, line 35
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line35
bq.  
bq.   What are these two booleans broken out?  Aren't they in they 
attributes of HRI already?  Why repeat them?

I used to put these transient parameters in the protobuff RegionInfo as well.  
However Todd thought it's better to put them outside.

What'd do you think?  To me, it is fine either way.  However, if we are going 
to replace HRI with the protobuff later on, it may be better to put them 
together.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionAdmin.proto, line 70
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line70
bq.  
bq.   Should we be repeating the API documentation that is up in the 
HRegionInterface that this .proto replaces here?  If its not here, where will 
it be?  Not all of the javadoc should make it over -- the stuff that says 
nothing shouldn't but some is of worth.  What you think?

I though about this.  I added documentation to RegionClient.proto.  For methods 
in RegionAdmin, the method names seem to be very clear.  I will take a look 
again and document where confusing/misunderstand could arise.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionAdmin.proto, line 87
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line87
bq.  
bq.   Isn't the response currently a void?   And isnt' flush async (IIRC). 
 If so, under what circumstance would we be able to fill out this response?

I was thinking to set it if the HRegion.flushCache call returned true.  It is 
just informational.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionAdmin.proto, line 112
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line112
bq.  
bq.   WALKey maps to HLogKey?  Maybe add a comment to this effect?

Sure.  Will add a comment.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionAdmin.proto, line 141
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line141
bq.  
bq.   Good.  I like this method name better.  Should be a comment which 
points back to the old name?  Or what you think?

Ok.  Will do.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionAdmin.proto, line 151
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95915#file95915line151
bq.  
bq.   Yeah, this proto is missing commentary.   I mean, the return here 
should be explained?

Ok, will add a comment too.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 52
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line52
bq.  
bq.   This is new?  Being able to do this?  How will it be used?

Some call expects a region name, some call expects an encoded region name.  
With a specifier, we can handle both.
Encoded region name works only if the region is on-line.  If we can't find the 
region based on the specifier, a region not found exception will be thrown.

So we can simplify the request a little bit since we don't have to ask for 
region name, and encoded region name, and check only if one is specified.

I will add a comment for it.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 66
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line66
bq.  
bq.   This is new feature on get?  Or just special handling of an 
attribute?

This is for the exist() call.  In this case, the caller doesn't care about the 
result. They just want to know if the row is there.  It is not special handling 
of an attribute.


bq.  On 2012-03-26 23:21:58, Michael Stack wrote:
bq.   src/main/proto/RegionClient.proto, line 71
bq.   https://reviews.apache.org/r/4054/diff/5/?file=95916#file95916line71
bq.

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-23 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13236846#comment-13236846
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review6294
---


Looks good overall.
Minor comments below.


src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13644

Is it possible that clusterIdLeastSignificantBits is present but 
clusterIdMostSignificantBits is absent ?



src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13646

Should this rpc be called getOnlineRegions ?



src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13647

Increment doesn't extend Mutation:

public class Increment implements Row {




src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13649

Remove 'is'



src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13651

white space.



src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13652

Should this line be removed ?



src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13653

'it's appeared' - 'it appears'



src/main/proto/RegionClient.proto
https://reviews.apache.org/r/4054/#comment13654

I don't see region member in Exec.java
Do we need this ?


- Ted


On 2012-03-22 20:20:39, 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-22 20:20:39)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  This is the first draft of the ProtoBuff HRegionProtocol.  The 
corresponding java vs pb method mapping is attached to the jira: 
https://issues.apache.org/jira/browse/HBASE-5443
bq.  
bq.  Please review.  I'd like to move ahead after we get to some agreement.
bq.  
bq.  
bq.  This addresses bug HBASE-5619.
bq.  https://issues.apache.org/jira/browse/HBASE-5619
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq.pom.xml 10b13ef 
bq.src/main/proto/RegionAdmin.proto PRE-CREATION 
bq.src/main/proto/RegionClient.proto PRE-CREATION 
bq.src/main/proto/hbase.proto PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/4054/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Jimmy
bq.  
bq.



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-23 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13236880#comment-13236880
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--



bq.  On 2012-03-23 17:54:17, Ted Yu wrote:
bq.   Looks good overall.
bq.   Minor comments below.

Thanks for reviewing.


bq.  On 2012-03-23 17:54:17, Ted Yu wrote:
bq.   src/main/proto/RegionAdmin.proto, line 115
bq.   https://reviews.apache.org/r/4054/diff/3/?file=94679#file94679line115
bq.  
bq.   Is it possible that clusterIdLeastSignificantBits is present but 
clusterIdMostSignificantBits is absent ?

They both should be either there, or not.  I can add an optional message UUID, 
which should have both listSigBits and mostSigBits. How is that?


bq.  On 2012-03-23 17:54:17, Ted Yu wrote:
bq.   src/main/proto/RegionClient.proto, line 147
bq.   https://reviews.apache.org/r/4054/diff/3/?file=94680#file94680line147
bq.  
bq.   Remove 'is'

Removed


bq.  On 2012-03-23 17:54:17, Ted Yu wrote:
bq.   src/main/proto/RegionClient.proto, line 153
bq.   https://reviews.apache.org/r/4054/diff/3/?file=94680#file94680line153
bq.  
bq.   white space.

Removed.


bq.  On 2012-03-23 17:54:17, Ted Yu wrote:
bq.   src/main/proto/RegionAdmin.proto, line 171
bq.   https://reviews.apache.org/r/4054/diff/3/?file=94679#file94679line171
bq.  
bq.   Should this rpc be called getOnlineRegions ?

For repeated parameter, for example, region, the generated method is something 
like getRegionList().  So it may be better to use singular for parameters.
For similar reason, I did the same for method name.  Another reason for that 
is, in the future, we may use this method to just get one online region given a 
region name.
This way, we can limit the number of methods.


bq.  On 2012-03-23 17:54:17, Ted Yu wrote:
bq.   src/main/proto/RegionClient.proto, line 139
bq.   https://reviews.apache.org/r/4054/diff/3/?file=94680#file94680line139
bq.  
bq.   Increment doesn't extend Mutation:
bq.   
bq.   public class Increment implements Row {
bq.  

That's right. But we can still make increment a mutation, in protocol buffer. 
Since Increment doesn't extend Mutation, it is a little bit different to map an 
Increment to a Mutate.
So we don't have to have a separate method for Increment.


bq.  On 2012-03-23 17:54:17, Ted Yu wrote:
bq.   src/main/proto/RegionClient.proto, line 171
bq.   https://reviews.apache.org/r/4054/diff/3/?file=94680#file94680line171
bq.  
bq.   Should this line be removed ?

Increment is treated as a Mutate, so it is needed.


bq.  On 2012-03-23 17:54:17, Ted Yu wrote:
bq.   src/main/proto/RegionClient.proto, line 208
bq.   https://reviews.apache.org/r/4054/diff/3/?file=94680#file94680line208
bq.  
bq.   'it's appeared' - 'it appears'

Fixed.


bq.  On 2012-03-23 17:54:17, Ted Yu wrote:
bq.   src/main/proto/RegionClient.proto, line 335
bq.   https://reviews.apache.org/r/4054/diff/3/?file=94680#file94680line335
bq.  
bq.   I don't see region member in Exec.java
bq.   Do we need this ?

I see. Let me enhance the doc.  The reason for that is Exec is called for a 
region.  In the request level, there is a default region.  Users can specify 
different region for each individual Exec.


- Jimmy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review6294
---


On 2012-03-22 20:20:39, 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-22 20:20:39)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  This is the first draft of the ProtoBuff HRegionProtocol.  The 
corresponding java vs pb method mapping is attached to the jira: 
https://issues.apache.org/jira/browse/HBASE-5443
bq.  
bq.  Please review.  I'd like to move ahead after we get to some agreement.
bq.  
bq.  
bq.  This addresses bug HBASE-5619.
bq.  https://issues.apache.org/jira/browse/HBASE-5619
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq.pom.xml 10b13ef 
bq.src/main/proto/RegionAdmin.proto PRE-CREATION 
bq.src/main/proto/RegionClient.proto PRE-CREATION 
bq.src/main/proto/hbase.proto PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/4054/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Jimmy
bq.  
bq.



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type:

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-23 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13236894#comment-13236894
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review6299
---



src/main/proto/RegionAdmin.proto
https://reviews.apache.org/r/4054/#comment13666

Introducing message UUID would be better.


- Ted


On 2012-03-22 20:20:39, 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-22 20:20:39)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  This is the first draft of the ProtoBuff HRegionProtocol.  The 
corresponding java vs pb method mapping is attached to the jira: 
https://issues.apache.org/jira/browse/HBASE-5443
bq.  
bq.  Please review.  I'd like to move ahead after we get to some agreement.
bq.  
bq.  
bq.  This addresses bug HBASE-5619.
bq.  https://issues.apache.org/jira/browse/HBASE-5619
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq.pom.xml 10b13ef 
bq.src/main/proto/RegionAdmin.proto PRE-CREATION 
bq.src/main/proto/RegionClient.proto PRE-CREATION 
bq.src/main/proto/hbase.proto PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/4054/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Jimmy
bq.  
bq.



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-23 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13236913#comment-13236913
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--



bq.  On 2012-03-23 18:30:08, Ted Yu wrote:
bq.   src/main/proto/RegionAdmin.proto, line 115
bq.   https://reviews.apache.org/r/4054/diff/3/?file=94679#file94679line115
bq.  
bq.   Introducing message UUID would be better.

Fixed.


- Jimmy


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/#review6299
---


On 2012-03-22 20:20:39, 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-22 20:20:39)
bq.  
bq.  
bq.  Review request for hbase.
bq.  
bq.  
bq.  Summary
bq.  ---
bq.  
bq.  This is the first draft of the ProtoBuff HRegionProtocol.  The 
corresponding java vs pb method mapping is attached to the jira: 
https://issues.apache.org/jira/browse/HBASE-5443
bq.  
bq.  Please review.  I'd like to move ahead after we get to some agreement.
bq.  
bq.  
bq.  This addresses bug HBASE-5619.
bq.  https://issues.apache.org/jira/browse/HBASE-5619
bq.  
bq.  
bq.  Diffs
bq.  -
bq.  
bq.pom.xml 10b13ef 
bq.src/main/proto/RegionAdmin.proto PRE-CREATION 
bq.src/main/proto/RegionClient.proto PRE-CREATION 
bq.src/main/proto/hbase.proto PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/4054/diff
bq.  
bq.  
bq.  Testing
bq.  ---
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Jimmy
bq.  
bq.



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-23 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13236975#comment-13236975
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/
---

(Updated 2012-03-23 19:29:52.180425)


Review request for hbase.


Changes
---

Addressed Ted's comments.


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-5619.
https://issues.apache.org/jira/browse/HBASE-5619


Diffs (updated)
-

  pom.xml 10b13ef 
  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



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-23 Thread Zhihong Yu (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13237034#comment-13237034
 ] 

Zhihong Yu commented on HBASE-5619:
---

@Stack, @Benoit:
Please take a look.

 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-22 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13235972#comment-13235972
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/
---

(Updated 2012-03-22 20:17:03.965254)


Review request for hbase.


Changes
---

HBASE-5443 is split into individual tasks. This one is about the protocol 
buffer definitions: HBASE-5619

Most of the review comments are addressed.


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-5619.
https://issues.apache.org/jira/browse/HBASE-5619


Diffs (updated)
-

  pom.xml 10b13ef 
  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



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-22 Thread jirapos...@reviews.apache.org (Commented) (JIRA)


[ 
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13235982#comment-13235982
 ] 

jirapos...@reviews.apache.org commented on HBASE-5619:
--


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4054/
---

(Updated 2012-03-22 20:20:39.056665)


Review request for hbase.


Summary (updated)
---

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-5619.
https://issues.apache.org/jira/browse/HBASE-5619


Diffs
-

  pom.xml 10b13ef 
  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



 Create PB protocols for HRegionInterface
 

 Key: HBASE-5619
 URL: https://issues.apache.org/jira/browse/HBASE-5619
 Project: HBase
  Issue Type: Sub-task
  Components: ipc, master, migration, regionserver
Reporter: Jimmy Xiang
Assignee: Jimmy Xiang
 Fix For: 0.96.0

 Attachments: hbase-5619.patch


 Subtask of HBase-5443, separate HRegionInterface into admin protocol and 
 client protocol, create the PB protocol buffer files

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (HBASE-5619) Create PB protocols for HRegionInterface

2012-03-22 Thread Hadoop QA (Commented) (JIRA)

[
https://issues.apache.org/jira/browse/HBASE-5619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13236043#comment-13236043
]

Hadoop QA commented on HBASE-5619:
--

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12519499/hbase-5619.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 javadoc. The javadoc tool did not generate any warning messages.

+1 javac. The applied patch does not increase the total number of javac
compiler warnings.

-1 findbugs. The patch appears to introduce 74 new Findbugs (version
1.3.9) warnings.

+1 release audit. The applied patch does not increase the total number of
release audit warnings.

-1 core tests. The patch failed these unit tests:

org.apache.hadoop.hbase.io.hfile.TestForceCacheImportantBlocks

Test results:
https://builds.apache.org/job/PreCommit-HBASE-Build/1263//testReport/
Findbugs warnings:
https://builds.apache.org/job/PreCommit-HBASE-Build/1263//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
Console output:
https://builds.apache.org/job/PreCommit-HBASE-Build/1263//console

This message is automatically generated.

Create PB protocols for HRegionInterface

Attachments: hbase-5619.patch

Subtask of HBase-5443, separate HRegionInterface into admin protocol and
client protocol, create the PB protocol buffer files

38 matches

Mail list logo