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/07 22:43:39 UTC
[kudu-CR] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Hello Jean-Daniel Cryans, Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/3329
to review the following change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
[java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
The testDisconnect test creates a single tablet table, scans the table,
disconnects a random tablet client, and then waits until the client has no
tablet clients. Since the Java client considers the master a tablet, this lasts
until the remaining tablet client times out. This commit changes the test to
instead disconnect all tablet clients, which shaves about 10 seconds off the
runtime. There are also a few drive-by fixups, including renaming
AsyncKuduClient.getTableClients to getTabletClients.
Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/kududb/client/ITClient.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
6 files changed, 14 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/3329/1
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 2:
Build Started http://104.196.14.100/job/kudu-gerrit/1767/
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3329/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 57: // 2. Disconnect the client.
We're disconnecting all of them, not just the one client we want to disconnect. That's why I was in favor of changing the method's name.
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 1:
Build Started http://104.196.14.100/job/kudu-gerrit/1762/
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3329/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 57: // 2. Disconnect the client.
> What would you like me to change it to?
disconnectAllClients or something like that. Could even move it to BaseKuduTest.
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 2: Code-Review-2
The test is now flaky. I think it's always been an issue, it's just only being exposed now that there isn't a 10 second wait.
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 1: -Verified
Build Started http://104.196.14.100/job/kudu-gerrit/1765/
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 1:
(1 comment)
Nice catch.
http://gerrit.cloudera.org:8080/#/c/3329/1/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
File java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java:
Line 93: for (TabletClient tabletClient : client.getTabletClients()) {
Instead, check that the port you get isn't the master's. BaseKuduTest offers findLeaderMasterPort(). You don't need to disconnect the master here. If you'd rather keep this code than change it the method's name and all the comments above such as "4. Disconnect the TS."
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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: Yes
[kudu-CR] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 5: -Code-Review
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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
Gerrit-HasComments: No
[kudu-CR] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3329/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 57: // 2. Disconnect the client.
> We're disconnecting all of them, not just the one client we want to disconn
What would you like me to change it to?
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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
Gerrit-HasComments: No
[kudu-CR] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
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/3329
to look at the new patch set (#2).
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
[java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
The testDisconnect test creates a single tablet table, scans the table,
disconnects a random tablet client, and then waits until the client has no
tablet clients. Since the Java client considers the master a tablet, this lasts
until the remaining tablet client times out. This commit changes the test to
instead disconnect all tablet clients, which shaves about 10 seconds off the
runtime. There are also a few drive-by fixups, including renaming
AsyncKuduClient.getTableClients to getTabletClients.
Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/kududb/client/ITClient.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
6 files changed, 16 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/3329/2
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 4:
Build Started http://104.196.14.100/job/kudu-gerrit/1772/
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has submitted this change and it was merged.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
[java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
The testDisconnect test creates a single tablet table, scans the table,
disconnects a random tablet client, and then waits until the client has no
tablet clients. Since the Java client considers the master a tablet, this lasts
until the remaining tablet client times out. This commit changes the test to
instead disconnect all tablet clients, which shaves about 10 seconds off the
runtime. There are also a few drive-by fixups, including renaming
AsyncKuduClient.getTableClients to getTabletClients.
The test fix exposed an existing race condition in the TabletClient
disconnect/reconnect handling which would cause an RPC to be 'lost' and never
be sent (eventually causing a timeout).
Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
Reviewed-on: http://gerrit.cloudera.org:8080/3329
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/kududb/client/ITClient.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
6 files changed, 17 insertions(+), 25 deletions(-)
Approvals:
Jean-Daniel Cryans: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
Gerrit-PatchSet: 6
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Hello Jean-Daniel Cryans, Adar Dembo, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/3329
to look at the new patch set (#3).
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
[java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
The testDisconnect test creates a single tablet table, scans the table,
disconnects a random tablet client, and then waits until the client has no
tablet clients. Since the Java client considers the master a tablet, this lasts
until the remaining tablet client times out. This commit changes the test to
instead disconnect all tablet clients, which shaves about 10 seconds off the
runtime. There are also a few drive-by fixups, including renaming
AsyncKuduClient.getTableClients to getTabletClients.
The test fix exposed an existing race condition in the TabletClient
disconnect/reconnect handling which would cause an RPC to be 'lost' and never
be sent (eventually causing a timeout).
Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
---
M java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
M java/kudu-client/src/test/java/org/kududb/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/kududb/client/ITClient.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
6 files changed, 17 insertions(+), 25 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/3329/3
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 3:
Build Started http://104.196.14.100/job/kudu-gerrit/1769/
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3329/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 57: // 2. Disconnect the client.
> Ah, so by client I mean AsyncKuduClient.
Ok fair enough.
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 5:
Build Started http://104.196.14.100/job/kudu-gerrit/1776/
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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
Gerrit-HasComments: No
[kudu-CR] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3329/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 57: // 2. Disconnect the client.
> disconnectAllClients or something like that. Could even move it to BaseKudu
Ah, so by client I mean AsyncKuduClient.
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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] [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [java-client] disconnect all tablets in TestAsyncKuduSession.disconnectAndWait
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3329/1/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java
File java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduClient.java:
Line 93: for (TabletClient tabletClient : client.getTabletClients()) {
> Instead, check that the port you get isn't the master's. BaseKuduTest offer
Not shutting down the master makes the while loop logic quite a bit more complicated. I've changed the comments. Not sure it really matters anyway, since it *is* disconnecting the TS.
--
To view, visit http://gerrit.cloudera.org:8080/3329
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I00b2530c63d70b37cc7143de9f6190f0fbf349de
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