[kudu-CR] [java-client] Fix a hang in TabletClient

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: [java-client] Fix a hang in TabletClient
..


[java-client] Fix a hang in TabletClient

TabletClient#sendRpc has been changing a lot lately, and it's finicky.
In this case we weren't handling RPCs that we needed to retry when the
connection had already been closed before we even tried to encode the
RPC. I've only been able to reproduce this on the cluster with really
slow inserts.

Change-Id: I5f92a2f9464f245182a59178f2eab8a88422864c
Reviewed-on: http://gerrit.cloudera.org:8080/3586
Reviewed-by: Dan Burkert 
Tested-by: Kudu Jenkins
---
M java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
1 file changed, 2 insertions(+), 1 deletion(-)

Approvals:
  Dan Burkert: Looks good to me, approved
  Kudu Jenkins: Verified



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

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

[kudu-CR] [java-client] Fix a hang in TabletClient

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: [java-client] Fix a hang in TabletClient
..


Patch Set 2:

Looks fine, but the cascading series of fixes to this code has left me with 
zero confidence that we got it right. Could you please add some automated test 
coverage? Unit tests would be ideal, but integration or stress tests would be 
fine too. Maybe a stress test where the client is aggressively disconnected 
from the servers, to exercise these code paths?

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

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

[kudu-CR] [java-client] Fix a hang in TabletClient

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [java-client] Fix a hang in TabletClient
..


Patch Set 2: Code-Review+2

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

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

[kudu-CR] [java-client] Fix a hang in TabletClient

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: [java-client] Fix a hang in TabletClient
..


Patch Set 2:

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

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

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

[kudu-CR] [java-client] Fix a hang in TabletClient

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: [java-client] Fix a hang in TabletClient
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3586/1//COMMIT_MSG
Commit Message:

Line 9: TabletClient#sendRpc has been changing a lot lately, and it's a finicky.
> s/a finicky/finicky
Done


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

PS1, Line 190: encodedRpcAndId can only be null if chan was null.
> I think this is more clear as 'encodedRpcAndId is null iff chan is null'
Done


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

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

[kudu-CR] [java-client] Fix a hang in TabletClient

[Dan Burkert (Code Review)]

Dan Burkert has posted comments on this change.

Change subject: [java-client] Fix a hang in TabletClient
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3586/1//COMMIT_MSG
Commit Message:

Line 9: TabletClient#sendRpc has been changing a lot lately, and it's a finicky.
s/a finicky/finicky


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

PS1, Line 190: encodedRpcAndId can only be null if chan was null.
I think this is more clear as 'encodedRpcAndId is null iff chan is null'


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

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

[kudu-CR] [java-client] Fix a hang in TabletClient

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has uploaded a new change for review.

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

Change subject: [java-client] Fix a hang in TabletClient
..

[java-client] Fix a hang in TabletClient

TabletClient#sendRpc has been changing a lot lately, and it's a finicky.
In this case we weren't handling RPCs that we needed to retry when the
connection had already been closed before we even tried to encode the
RPC. I've only been able to reproduce this on the cluster with really
slow inserts.

Change-Id: I5f92a2f9464f245182a59178f2eab8a88422864c
---
M java/kudu-client/src/main/java/org/kududb/client/TabletClient.java
1 file changed, 2 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5f92a2f9464f245182a59178f2eab8a88422864c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans