You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Binglin Chang (Code Review)" <ge...@cloudera.org> on 2016/06/03 03:35:44 UTC
[kudu-CR] [java client] Fix GetTableLocations error handling
Binglin Chang has uploaded a new change for review.
http://gerrit.cloudera.org:8080/3295
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
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
2 files changed, 30 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/3295/1
--
To view, visit http://gerrit.cloudera.org:8080/3295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I79d246a83e9f0071063a23fb9a8d4b4c0220e507
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Binglin Chang <de...@gmail.com>
[kudu-CR] [java client] Fix GetTableLocations error handling
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: [java client] Fix GetTableLocations error handling
......................................................................
Patch Set 1:
Build Started http://104.196.14.100/job/kudu-gerrit/1730/
--
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: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Binglin Chang <de...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] [java client] Fix GetTableLocations error handling
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
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 <de...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] [java client] Fix GetTableLocations error handling
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Fix GetTableLocations error handling
......................................................................
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/3295/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 764: * "Errback" used to delayed-retry a RPC if it fails due to no leader master being found.
Javadoc needs to be modified.
Line 793: request.errback(arg);
I was surprised originally to see this issue, but it makes sense. The errback in this case is added on the GetTableLocationsResponse, not the actual RPC. I wonder if we have other cases like this where we assume we are sending the errback all the way to the client., but in reality they get lost.
http://gerrit.cloudera.org:8080/#/c/3295/2/java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
File java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java:
PS2, Line 113: RPC get stuck.
nit: to* get stuck.
Line 119: session.setFlushMode(AsyncKuduSession.FlushMode.AUTO_FLUSH_SYNC);
Isn't that line unnecessary?
Line 126: Thread.sleep(2000);
Can we find another way to find when the tablet is deleted to not sleep for 2 seconds?
PS2, Line 131: cause table is deleted.
nit: because the table was deleted.
Line 132: e.printStackTrace();
Why print it if it's expected?
--
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Binglin Chang <de...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Fix GetTableLocations error handling
Posted by "Binglin Chang (Code Review)" <ge...@cloudera.org>.
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 (#3).
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, 159 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/3295/3
--
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: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Binglin Chang <de...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Fix GetTableLocations error handling
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] 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
Reviewed-on: http://gerrit.cloudera.org:8080/3295
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans
---
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(-)
Approvals:
Jean-Daniel Cryans: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/3295
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I79d246a83e9f0071063a23fb9a8d4b4c0220e507
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Binglin Chang <de...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Fix GetTableLocations error handling
Posted by "Binglin Chang (Code Review)" <ge...@cloudera.org>.
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 <de...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java client] Fix GetTableLocations error handling
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java client] Fix GetTableLocations error handling
......................................................................
Patch Set 4: Code-Review+2
--
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 <de...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] [java client] Fix GetTableLocations error handling
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
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 <de...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java client] Fix GetTableLocations error handling
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: [java client] Fix GetTableLocations error handling
......................................................................
Patch Set 3:
Build Started http://104.196.14.100/job/kudu-gerrit/1822/
--
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 <de...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] [java client] Fix GetTableLocations error handling
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: [java client] Fix GetTableLocations error handling
......................................................................
Patch Set 2:
Build Started http://104.196.14.100/job/kudu-gerrit/1731/
--
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Binglin Chang <de...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] [java client] Fix GetTableLocations error handling
Posted by "Binglin Chang (Code Review)" <ge...@cloudera.org>.
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 (#2).
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
M java/kudu-client/src/test/java/org/kududb/client/TestAsyncKuduSession.java
2 files changed, 34 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/95/3295/2
--
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: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Binglin Chang <de...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins