You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Dan Burkert (Code Review)" <ge...@cloudera.org> on 2016/06/03 18:19:46 UTC
[kudu-CR] Update Java client for new master GetTableLocations semantics
Hello Jean-Daniel Cryans, Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/3303
to review the following change.
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, 45 insertions(+), 30 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/3303/1
--
To view, visit http://gerrit.cloudera.org:8080/3303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I80bf5661aed1ba435800211411b3273874e5bbcf
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 4: Code-Review+2
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 4:
Build Started http://104.196.14.100/job/kudu-gerrit/1833/
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/3303/3/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:
PS3, Line 666:
> to the leader master instance *since it's also handled like a tablet.
Done
http://gerrit.cloudera.org:8080/#/c/3303/3/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
File java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java:
Line 155: }
> Remind me why you don't need all those tests you previously were adding?
I realized they already existed TestKuduTable.java.
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 1:
I think this may be a breaking change. Still thinking about how to do it backwards compatibly.
--
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/3303/3/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:
PS3, Line 666: .
to the leader master instance *since it's also handled like a tablet.
http://gerrit.cloudera.org:8080/#/c/3303/3/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
File java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java:
Line 155: }
Remind me why you don't need all those tests you previously were adding?
--
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 1:
This change will cause a 1.0 Java client to misinterpret an empty GetTableLocations response from a 0.9 master as a non-covered range (empty table), when in reality it indicates that the table has non-running tablets.
--
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 3:
Build Started http://104.196.14.100/job/kudu-gerrit/1828/
--
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 1:
(1 comment)
It would be great if we could test the modified code paths.
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 locations", right? If so, could you add a comment explaining that?
--
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 1:
(1 comment)
> This change will cause a 1.0 Java client to misinterpret an empty
> GetTableLocations response from a 0.9 master as a non-covered range
> (empty table), when in reality it indicates that the table has
> non-running tablets.
Add release notes?
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 664: * requests to the leader master instance.
since the master is considered a tablet.
--
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 4: -Verified
Build Started http://104.196.14.100/job/kudu-gerrit/1859/
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 4: Code-Review+2
Much cleaner, this was a good idea.
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 1:
Build Started http://104.196.14.100/job/kudu-gerrit/1737/
--
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
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 (#4).
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/4
--
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: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: Update Java client for new master GetTableLocations semantics
......................................................................
Patch Set 2:
Build Started http://104.196.14.100/job/kudu-gerrit/1825/
--
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 <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] Update Java client for new master GetTableLocations semantics
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has submitted this change and it was merged.
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
Reviewed-on: http://gerrit.cloudera.org:8080/3303
Reviewed-by: Jean-Daniel Cryans
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
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(-)
Approvals:
Jean-Daniel Cryans: Looks good to me, approved
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/3303
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I80bf5661aed1ba435800211411b3273874e5bbcf
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins