Matthew Jacobs has posted comments on this change. Change subject: IMPALA-5558: Reopen stale client connection ......................................................................
Patch Set 7: (5 comments) Did a first pass, I think it makes sense but I'll need to think a bit more about it. I'll look again with fresh eyes later or in the morning. I'm still cautious of doing this vs reverting the prev patches, but that needs more thought as well. http://gerrit.cloudera.org:8080/#/c/7284/7//COMMIT_MSG Commit Message: Line 7: IMPALA-5558: Reopen stale client connection you can also say this fixes IMPALA-5576 PS7, Line 40: cncelled typo PS7, Line 36: This change also fixes QueryState::ReportExecStatusAux() to unconditionally : for up to 3 times when reporting exec status of a fragment instance. Previously, : it may break out of the loop early if RPC fails with 'retry_is_safe' == true : (e.g. due to stale connections above) or if the connection to coordinator fails. : Failing the RPC may cause all fragment instances of a query to be cncelled locally, : triggering query hang due to IMPALA-2990. The status reporting is idempotent as : the handler simply ignores profiles of fragment instances which are done already : so it should be retried uncontionally up to 3 times. Similarly, the cancellation : RPC is also idempotent so it should be unconditionally retried up to 3 times. nit: wrap http://gerrit.cloudera.org:8080/#/c/7284/7/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: PS7, Line 347: if (i < 2) SleepForMs(100); why? http://gerrit.cloudera.org:8080/#/c/7284/7/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: PS7, Line 237: !client_status.ok() I think that adding this here is essentially equivalent to the bug IMPALA-5576, no? -- To view, visit http://gerrit.cloudera.org:8080/7284 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4d722c8ad3bf0e78e89887b6cb286c69ca61b8f5 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Ho <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Juan Yu <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: Yes
