[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Henry Robinson has submitted this change and it was merged. Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. IMPALA-3682: Don't retry unrecoverable socket creation errors If a thrift client can't create a socket, all subsequent calls to Open() should fail fast since socket creation errors are treated as unrecoverable. Testing: manual testing with a bad SSL configuration. Impalad startup fails fast, rather than retrying 10 times as previously. Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Reviewed-on: http://gerrit.cloudera.org:8080/3317 Reviewed-by: Dan Hecht Tested-by: Henry Robinson --- M be/src/rpc/thrift-client.cc M be/src/rpc/thrift-server-test.cc M be/src/runtime/exec-env.cc M common/thrift/generate_error_codes.py 4 files changed, 56 insertions(+), 19 deletions(-) Approvals: Henry Robinson: Verified Dan Hecht: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 3 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Henry Robinson has posted comments on this change. Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. Patch Set 2: -Code-Review Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Henry Robinson has posted comments on this change. Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. Patch Set 2: Code-Review+1 Code build passed in this job: http://sandbox.jenkins.cloudera.com/job/impala-umbrella-build-and-test/1726/ -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Dan Hecht has posted comments on this change. Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. Patch Set 2: Code-Review+2 Great, thanks. -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Hello Sailesh Mukil, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3317 to look at the new patch set (#2). Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. IMPALA-3682: Don't retry unrecoverable socket creation errors If a thrift client can't create a socket, all subsequent calls to Open() should fail fast since socket creation errors are treated as unrecoverable. Testing: manual testing with a bad SSL configuration. Impalad startup fails fast, rather than retrying 10 times as previously. Change-Id: I394be287143eefc79cf22865898b71ca24c41328 --- M be/src/rpc/thrift-client.cc M be/src/rpc/thrift-server-test.cc M be/src/runtime/exec-env.cc M common/thrift/generate_error_codes.py 4 files changed, 56 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/17/3317/2 -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Henry Robinson has posted comments on this change. Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. Patch Set 2: I added a test case that exercises that path. Doesn't test this change explicitly, but confirms the correct behavior is maintained. -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Dan Hecht has posted comments on this change. Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. Patch Set 1: > Hard to add automated testing, partly because the eventual behavior > hasn't changed, just the number of retries it takes to get there. I > did manually test this with a bad SSL setup, and saw a fast failure > as expected. I've added this to the commit message. Yeah, I can see why testing the old vs new is tough, but do we have an automated test to exercise this path? -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Henry Robinson has posted comments on this change. Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. Patch Set 1: Hard to add automated testing, partly because the eventual behavior hasn't changed, just the number of retries it takes to get there. I did manually test this with a bad SSL setup, and saw a fast failure as expected. I've added this to the commit message. -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Dan Hecht has posted comments on this change. Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. Patch Set 1: Code-Review+2 Any testing we could add for this? -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3317/1/be/src/rpc/thrift-client.cc File be/src/rpc/thrift-client.cc: PS1, Line 36: if (!socket_create_status_.ok()) return socket_create_status_; > Even if not, we should be defensive against the possibility that someone wi Makes sense. -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Henry Robinson has posted comments on this change. Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3317/1/be/src/rpc/thrift-client.cc File be/src/rpc/thrift-client.cc: PS1, Line 36: if (!socket_create_status_.ok()) return socket_create_status_; > Any reason to still leave this here now? I can't recall if direct calls to Even if not, we should be defensive against the possibility that someone will try to call Open() again in the future (and if we don't think they should, we should make that private). -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Sailesh Mukil has posted comments on this change. Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/3317/1/be/src/rpc/thrift-client.cc File be/src/rpc/thrift-client.cc: PS1, Line 36: if (!socket_create_status_.ok()) return socket_create_status_; Any reason to still leave this here now? I can't recall if direct calls to Open() are made. -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson Gerrit-Reviewer: Sailesh Mukil Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3682: Don't retry unrecoverable socket creation errors
Henry Robinson has uploaded a new change for review. http://gerrit.cloudera.org:8080/3317 Change subject: IMPALA-3682: Don't retry unrecoverable socket creation errors .. IMPALA-3682: Don't retry unrecoverable socket creation errors If a thrift client can't create a socket, all subsequent calls to Open() should fail fast since socket creation errors are treated as unrecoverable. Change-Id: I394be287143eefc79cf22865898b71ca24c41328 --- M be/src/rpc/thrift-client.cc M be/src/runtime/exec-env.cc M common/thrift/generate_error_codes.py 3 files changed, 6 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/17/3317/1 -- To view, visit http://gerrit.cloudera.org:8080/3317 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I394be287143eefc79cf22865898b71ca24c41328 Gerrit-PatchSet: 1 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson
