You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/03/03 03:18:42 UTC
[kudu-CR] KUDU-1906. Fix lost callback for scanner path
Hello Jean-Daniel Cryans,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/6239
to review the following change.
Change subject: KUDU-1906. Fix lost callback for scanner path
......................................................................
KUDU-1906. Fix lost callback for scanner path
Fixes another case similar to KUDU-1888 in which we were sending an RPC
before setting its deferred. In the case that the RPC responded very
quickly, the response would come before the callback was attached, and
the callback would never get called.
This caused my RowCounter jobs on a small/underpowered test cluster to
have task timeouts a few percent of the time.
This patch fixes the particular instance and also adds some assertions
to try to prevent this style of bug in the future.
Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
3 files changed, 8 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/6239/1
--
To view, visit http://gerrit.cloudera.org:8080/6239
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
[kudu-CR] KUDU-1906. Fix lost callback for scanner path
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has submitted this change and it was merged.
Change subject: KUDU-1906. Fix lost callback for scanner path
......................................................................
KUDU-1906. Fix lost callback for scanner path
Fixes another case similar to KUDU-1888 in which we were sending an RPC
before setting its deferred. In the case that the RPC responded very
quickly, the response would come before the callback was attached, and
the callback would never get called.
This caused my RowCounter jobs on a small/underpowered test cluster to
have task timeouts a few percent of the time.
This patch fixes the particular instance and also adds some assertions
to try to prevent this style of bug in the future.
Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Reviewed-on: http://gerrit.cloudera.org:8080/6239
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
4 files changed, 10 insertions(+), 2 deletions(-)
Approvals:
Jean-Daniel Cryans: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/6239
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-1906. Fix lost callback for scanner path
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: KUDU-1906. Fix lost callback for scanner path
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6239
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] KUDU-1906. Fix lost callback for scanner path
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: KUDU-1906. Fix lost callback for scanner path
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6239
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] KUDU-1906. Fix lost callback for scanner path
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Jean-Daniel Cryans, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6239
to look at the new patch set (#2).
Change subject: KUDU-1906. Fix lost callback for scanner path
......................................................................
KUDU-1906. Fix lost callback for scanner path
Fixes another case similar to KUDU-1888 in which we were sending an RPC
before setting its deferred. In the case that the RPC responded very
quickly, the response would come before the callback was attached, and
the callback would never get called.
This caused my RowCounter jobs on a small/underpowered test cluster to
have task timeouts a few percent of the time.
This patch fixes the particular instance and also adds some assertions
to try to prevent this style of bug in the future.
Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
M java/kudu-client/src/main/java/org/apache/kudu/client/TabletClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
4 files changed, 10 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/6239/2
--
To view, visit http://gerrit.cloudera.org:8080/6239
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I102778e87d0f153cdd2a1ca2aed3ec1e17014d4b
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins