[kudu-CR] [java client] Fix GetTableLocations error handling

2016-06-14 Thread Kudu Jenkins (Code Review) 
Kudu Jenkins has posted comments on this change.

Change subject: [java client] Fix GetTableLocations error handling
..


Patch Set 4:

Build Started http://104.196.14.100/job/kudu-gerrit/1830/

-- 
To view, visit http://gerrit.cloudera.org:8080/3295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I79d246a83e9f0071063a23fb9a8d4b4c0220e507
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Binglin Chang 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] [java client] Fix GetTableLocations error handling

2016-06-14 Thread Binglin Chang (Code Review) 
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/3295

to look at the new patch set (#4).

Change subject: [java client] Fix GetTableLocations error handling
..

[java client] Fix GetTableLocations error handling

When GetTableLocations get an error, e.g. table not found, in some
scenario the callback will silently drop the error, not notifying
the RPC which triggered this GetTableLocations call, causing the
RPC to hang.

This usually happens when user is inserting data, but underlying
table is deleted and recreated.

Change-Id: I79d246a83e9f0071063a23fb9a8d4b4c0220e507
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
A java/kudu-client/src/main/java/org/kududb/client/ListTabletsRequest.java
A java/kudu-client/src/main/java/org/kududb/client/ListTabletsResponse.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
4 files changed, 157 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/3295/4
-- 
To view, visit http://gerrit.cloudera.org:8080/3295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I79d246a83e9f0071063a23fb9a8d4b4c0220e507
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Binglin Chang 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] Update Java client for new master GetTableLocations semantics

2016-06-14 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: Update Java client for new master GetTableLocations semantics
..


Patch Set 2:

(1 comment)

The changes I made today have been made significantly simpler by building on 
top of https://gerrit.cloudera.org/#/c/3386/.  I don't think any of the review 
comments still apply.  There still isn't a lot of explicit testing of this 
change, except that these codepaths are hit in pretty much every operation 
involving a tablet.

http://gerrit.cloudera.org:8080/#/c/3303/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 1134:   if (response.getError().getCode() == 
Master.MasterErrorPB.Code.TABLET_NOT_RUNNING) {
> IIUC you're relying on existing tests to pass to verify this new check?
No, the new tests will hit this codepath in the case where the tablets are not 
yet created when we start calling syncLocateTable.  The behavior should here is 
just a retry, so no special logic is needed in the tests themselves.


-- 
To view, visit http://gerrit.cloudera.org:8080/3303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80bf5661aed1ba435800211411b3273874e5bbcf
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] [java-client] use tablet cache for locateTablet calls

2016-06-14 Thread Kudu Jenkins (Code Review) 
Kudu Jenkins has posted comments on this change.

Change subject: [java-client] use tablet cache for locateTablet calls
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/1829/

-- 
To view, visit http://gerrit.cloudera.org:8080/3386
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I78b5d400778547a9ee090111663677901dbadd98
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Update Java client for new master GetTableLocations semantics

2016-06-14 Thread Dan Burkert (Code Review) 
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/3303

to look at the new patch set (#3).

Change subject: Update Java client for new master GetTableLocations semantics
..

Update Java client for new master GetTableLocations semantics

The Java client was relying on the master returning an empty tablet locations
response when the table was still in the process of being created. Now, the
master returns a specific error. This code fixes the table locations lookup code
in AsyncKuduClient for look for that specific error code.

This change isn't tested since the locateTablets codepath is well covered by
existing tests.

Change-Id: I80bf5661aed1ba435800211411b3273874e5bbcf
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
2 files changed, 45 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/3303/3
-- 
To view, visit http://gerrit.cloudera.org:8080/3303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I80bf5661aed1ba435800211411b3273874e5bbcf
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java-client] use tablet cache for locateTablet calls

2016-06-14 Thread Dan Burkert (Code Review) 
Dan Burkert has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/3386

Change subject: [java-client] use tablet cache for locateTablet calls
..

[java-client] use tablet cache for locateTablet calls

This simplifies and improves the performance of AsyncKuduClient::locateTable by
using the client's existing tablet cache. The semantics of
locateTable are changed slightly- the end key is now treated as an
exclusive key instead of inclusive. This is a breaking change for the public
KuduTable::locateTable methods. These methods are a holdover from the pre-scan
token era, and should almost certainly be deprecated. To use correctly they
require great knowledge of Kudu's internal partitioning logic.

I'm not too worried about making this breaking change since the API is pretty
much impossible to use to begin with, and no one is using it (except for the
scan tokens impl, and it was already treating the end key as exclusive).

Change-Id: I78b5d400778547a9ee090111663677901dbadd98
---
M docs/release_notes.adoc
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/KuduTable.java
M java/kudu-client/src/main/java/org/kududb/client/LocatedTablet.java
M java/kudu-client/src/test/java/org/kududb/client/TestKuduTable.java
5 files changed, 109 insertions(+), 86 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/86/3386/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3386
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I78b5d400778547a9ee090111663677901dbadd98
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert

[kudu-CR] Improvements and corrections to Impala CREATE TABLE examples

2016-06-14 Thread Casey Ching (Code Review) 
Casey Ching has posted comments on this change.

Change subject: Improvements and corrections to Impala CREATE TABLE examples
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3376/1/docs/kudu_impala_integration.adoc
File docs/kudu_impala_integration.adoc:

Line 501: DISTRIBUTE BY HASH (id) INTO 4 BUCKETS
> Does Impala allow this?  We have been moving away from any kind of implicit
Alright that's fine with me. Just filed 
https://issues.cloudera.org/browse/IMPALA-3747 for the change.


-- 
To view, visit http://gerrit.cloudera.org:8080/3376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I093972a7b806787a8c72634851796eebb5e1ae4c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Misty Stanley-Jones 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Casey Ching 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Misty Stanley-Jones 
Gerrit-HasComments: Yes

[kudu-CR] Update Java client for new master GetTableLocations semantics

2016-06-14 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change.

Change subject: Update Java client for new master GetTableLocations semantics
..


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/3303/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 1135: // The table is most likely still being create.
Nit: 'created' was correct.


Line 1136: LOG.debug("Table {} has a non-running tablet", 
tableId);
Does this automatically include backoff?


Line 1137: return loopLocateTable(tableId, startPartitionKey, 
endPartitionKey,
Is there an upper bound on the recursive depth here? I suppose we already had 
that problem before (L1164-1165) but I'm curious to know whether either of 
these are safe.


http://gerrit.cloudera.org:8080/#/c/3303/2/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
File java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java:

Line 249: assertTrue("unreachable", false);
> use fail()
Better yet, put it inside the try {} itself after  syncLocateTable(). Then you 
don't need to return from the catch.


-- 
To view, visit http://gerrit.cloudera.org:8080/3303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80bf5661aed1ba435800211411b3273874e5bbcf
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Improvements and corrections to Impala CREATE TABLE examples

2016-06-14 Thread Misty Stanley-Jones (Code Review) 
Hello Casey Ching, Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/3376

to look at the new patch set (#2).

Change subject: Improvements and corrections to Impala CREATE TABLE examples
..

Improvements and corrections to Impala CREATE TABLE examples

Change-Id: I093972a7b806787a8c72634851796eebb5e1ae4c
---
M docs/kudu_impala_integration.adoc
1 file changed, 18 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/3376/2
-- 
To view, visit http://gerrit.cloudera.org:8080/3376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I093972a7b806787a8c72634851796eebb5e1ae4c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Misty Stanley-Jones 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Casey Ching 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Misty Stanley-Jones

[kudu-CR] Improvements and corrections to Impala CREATE TABLE examples

2016-06-14 Thread Kudu Jenkins (Code Review) 
Kudu Jenkins has posted comments on this change.

Change subject: Improvements and corrections to Impala CREATE TABLE examples
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/1827/

-- 
To view, visit http://gerrit.cloudera.org:8080/3376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I093972a7b806787a8c72634851796eebb5e1ae4c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Misty Stanley-Jones 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Casey Ching 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Misty Stanley-Jones 
Gerrit-HasComments: No

[kudu-CR] Improvements and corrections to Impala CREATE TABLE examples

2016-06-14 Thread Misty Stanley-Jones (Code Review) 
Misty Stanley-Jones has posted comments on this change.

Change subject: Improvements and corrections to Impala CREATE TABLE examples
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3376/1/docs/kudu_impala_integration.adoc
File docs/kudu_impala_integration.adoc:

Line 407: DISTRIBUTE BY HASH(ts) INTO 16 BUCKETS
> Nit: separate "HASH" and "(ts)" with a space.
Done


Line 501: DISTRIBUTE BY HASH (id) INTO 4 BUCKETS
> The "(id)" isn't needed here, all key columns is the default. Maybe remove 
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3376
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I093972a7b806787a8c72634851796eebb5e1ae4c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Misty Stanley-Jones 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Casey Ching 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Misty Stanley-Jones 
Gerrit-HasComments: Yes

[kudu-CR] ubsan: use our own libstdcxx

2016-06-14 Thread Kudu Jenkins (Code Review) 
Kudu Jenkins has posted comments on this change.

Change subject: ubsan: use our own libstdcxx
..


Patch Set 2:

Build Started http://104.196.14.100/job/kudu-gerrit/1826/

-- 
To view, visit http://gerrit.cloudera.org:8080/2904
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I54f49d808aeb97d93d247a46a93161a1634921de
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Update Java client for new master GetTableLocations semantics

2016-06-14 Thread Jean-Daniel Cryans (Code Review) 
Jean-Daniel Cryans has posted comments on this change.

Change subject: Update Java client for new master GetTableLocations semantics
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3303/2/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 1134:   if (response.getError().getCode() == 
Master.MasterErrorPB.Code.TABLET_NOT_RUNNING) {
IIUC you're relying on existing tests to pass to verify this new check?


http://gerrit.cloudera.org:8080/#/c/3303/2/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
File java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java:

Line 203:new byte[] 
{ (byte) 0x80, 0x00, 0x00, 0x32 },
Got some long lines here and below.


Line 249: assertTrue("unreachable", false);
use fail()


-- 
To view, visit http://gerrit.cloudera.org:8080/3303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80bf5661aed1ba435800211411b3273874e5bbcf
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Update Java client for new master GetTableLocations semantics

2016-06-14 Thread Dan Burkert (Code Review) 
Dan Burkert has posted comments on this change.

Change subject: Update Java client for new master GetTableLocations semantics
..


Patch Set 1:

(1 comment)

Updated to make the similar change for the locate tablets codepath, since it's 
pretty much duplicated logic.  I added tests for locate tablets, I think the 
codepath for normal tablet lookup is quite well tested by existing tests 
(pretty much everything has to locate tablets).

http://gerrit.cloudera.org:8080/#/c/3303/1/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 1045: new GetTableLocationsRequest(masterTable, partitionKey, 
null, tableId);
> This is more of the "leave the end key blank so we'll read ahead other loca
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I80bf5661aed1ba435800211411b3273874e5bbcf
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] Update Java client for new master GetTableLocations semantics

2016-06-14 Thread Dan Burkert (Code Review) 
Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/3303

to look at the new patch set (#2).

Change subject: Update Java client for new master GetTableLocations semantics
..

Update Java client for new master GetTableLocations semantics

The Java client was relying on the master returning an empty tablet locations
response when the table was still in the process of being created. Now, the 
master
returns a specific error. This code fixes the table locations lookup code in
AsyncKuduClient for look for that specific error code.

Change-Id: I80bf5661aed1ba435800211411b3273874e5bbcf
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
2 files changed, 159 insertions(+), 39 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/3303/2
-- 
To view, visit http://gerrit.cloudera.org:8080/3303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I80bf5661aed1ba435800211411b3273874e5bbcf
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [java client] Fix GetTableLocations error handling

2016-06-14 Thread Jean-Daniel Cryans (Code Review) 
Jean-Daniel Cryans has posted comments on this change.

Change subject: [java client] Fix GetTableLocations error handling
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3295/3/java/kudu-client/src/main/java/org/kududb/client/ListTabletsResponse.java
File java/kudu-client/src/main/java/org/kududb/client/ListTabletsResponse.java:

Line 24: @InterfaceAudience.Public
I would mark it private since we're not exposing anyway of using it.


PS3, Line 39: sL
nit: Tablet*


-- 
To view, visit http://gerrit.cloudera.org:8080/3295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I79d246a83e9f0071063a23fb9a8d4b4c0220e507
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Binglin Chang 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes

[kudu-CR] ITBLL: Specify all possible columns to partition on

2016-06-14 Thread Jean-Daniel Cryans (Code Review) 
Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: ITBLL: Specify all possible columns to partition on
..


ITBLL: Specify all possible columns to partition on

key2 may be used as a partitioning column as well, if more than one
tablet is specified.

Change-Id: Ibf18771f7d2dc6e070a792ccfdd1f4069a3b0d91
Reviewed-on: http://gerrit.cloudera.org:8080/3380
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans
---
M 
java/kudu-client-tools/src/main/java/org/kududb/mapreduce/tools/IntegrationTestBigLinkedList.java
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/3380
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ibf18771f7d2dc6e070a792ccfdd1f4069a3b0d91
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins