Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14981 )
Change subject: [java] retry to connect to the cluster when specifing a superset of masters ...................................................................... Patch Set 2: (3 comments) Will you be making an equivalent change to the C++ client, either here or in a separate patch? http://gerrit.cloudera.org:8080/#/c/14981/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14981/1//COMMIT_MSG@9 PS1, Line 9: When the user specifies more masters in the connection string than the actual : existing masters, but couldn't find a leader master, the client should retry : to connect to the cluster instead of returning with NonRecoverableException. : This will be useful when we need to add or remove masters. : > Maybe I didn't make it clear. This patch want to achieve when the client is You made it clear; I misread the change to ConnectToCluster.java and interpreted it backwards. It does what you say it does: if the client believes there are more masters than the cluster says there are, we now treat that as a retryable condition. Sorry about that. http://gerrit.cloudera.org:8080/#/c/14981/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14981/2//COMMIT_MSG@13 PS2, Line 13: ` Snuck in by mistake? http://gerrit.cloudera.org:8080/#/c/14981/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java: http://gerrit.cloudera.org:8080/#/c/14981/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java@225 PS2, Line 225: // Test the client try to connect three masters, : // but the cluster is configure with only one master. : : // Connect to one leader master, success. : runTest( : reusableNRE, : reusableNRE, : makeCTMR(LEADER, ImmutableList.of(MASTERS.get(0))), : successResponse); : : // The master hasn't become a leader, retry. : runTest( : reusableNRE, : reusableNRE, : makeCTMR(FOLLOWER, ImmutableList.of(MASTERS.get(0))), : retryResponse); Nit: could you move these so they're part of the existing "success", "retry", or "failure" blocks of the test? -- To view, visit http://gerrit.cloudera.org:8080/14981 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I66033c3ff6d217ce6b8286c94a7333b90cd26d19 Gerrit-Change-Number: 14981 Gerrit-PatchSet: 2 Gerrit-Owner: Yifan Zhang <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Comment-Date: Wed, 08 Jan 2020 18:05:12 +0000 Gerrit-HasComments: Yes
