[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. IMPALA-9756: avoid coord cancellation/unregistration races This patch aims to make the query tear-down path more predictable. Specifically, we want to ensure that Coordinator::ComputeQuerySummary() finishes before the query is unregistered. Before that could happen if cancellation occurred on a different thread from unregistration, e.g. if async_exec_thread_ cancels the query, if an asynchronous Cancel() RPC comes in, if it was cancelled internally because of a timeout, or for some other reason. The race was possible for a couple of reasons: * The synchronization between threads happened on UpdateExecState(), which is called before ComputeQuerySummary(). * The unregistering thread may not call Coordinator::Cancel() if coord_exec_called_ is not set. This patch addresses both problems, by adding synchronization so that Coordinator::Cancel() blocks until ComputeQuerySummary() finishes, and so that coord_exec_called_ is set even if 'async_exec_thread_' is cancelling the query (which ensures that both threads call Coordinator::Cancel() and synchronize there). This is only done on the finalizing thread. Other threads that initiate cancellation should not block on cancellation, so that threads cannot pile up waiting for the query. Testing: Added a regression test that failed before this change. Ran exhaustive tests. Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Reviewed-on: http://gerrit.cloudera.org:8080/15954 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M tests/util/cancel_util.py 5 files changed, 57 insertions(+), 16 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 9 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 May 2020 22:10:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5881/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 May 2020 16:58:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 8: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 8 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 May 2020 16:58:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 7: Hit IMPALA-9751 -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 May 2020 16:01:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5874/ -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 May 2020 09:17:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5874/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 May 2020 03:51:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 May 2020 03:51:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 6: (1 comment) Unrelated plannerTest failure. http://gerrit.cloudera.org:8080/#/c/15954/4/be/src/runtime/coordinator.h File be/src/runtime/coordinator.h: http://gerrit.cloudera.org:8080/#/c/15954/4/be/src/runtime/coordinator.h@101 PS4, Line 101: /// above is reached (error, cancelled, or EOS) to ensure resources are released. Update comment. -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 21 May 2020 03:51:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 6: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5872/ -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 21 May 2020 02:28:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 20 May 2020 21:10:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5872/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 6 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 20 May 2020 21:10:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 20 May 2020 21:05:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/6118/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 20 May 2020 19:30:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/15954/5/be/src/service/client-request-state.h File be/src/service/client-request-state.h: http://gerrit.cloudera.org:8080/#/c/15954/5/be/src/service/client-request-state.h@182 PS5, Line 182: Status Cancel(bool check_inflight, const Status* cause, bool wait_until_finalized=false); line too long (91 > 90) http://gerrit.cloudera.org:8080/#/c/15954/5/tests/util/cancel_util.py File tests/util/cancel_util.py: http://gerrit.cloudera.org:8080/#/c/15954/5/tests/util/cancel_util.py@67 PS5, Line 67: ( flake8: E129 visually indented line with same indent as next logical line -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 20 May 2020 18:47:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9756: avoid coord cancellation/unregistration races
Tim Armstrong has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/15954 ) Change subject: IMPALA-9756: avoid coord cancellation/unregistration races .. IMPALA-9756: avoid coord cancellation/unregistration races This patch aims to make the query tear-down path more predictable. Specifically, we want to ensure that Coordinator::ComputeQuerySummary() finishes before the query is unregistered. Before that could happen if cancellation occurred on a different thread from unregistration, e.g. if async_exec_thread_ cancels the query, if an asynchronous Cancel() RPC comes in, if it was cancelled internally because of a timeout, or for some other reason. The race was possible for a couple of reasons: * The synchronization between threads happened on UpdateExecState(), which is called before ComputeQuerySummary(). * The unregistering thread may not call Coordinator::Cancel() if coord_exec_called_ is not set. This patch addresses both problems, by adding synchronization so that Coordinator::Cancel() blocks until ComputeQuerySummary() finishes, and so that coord_exec_called_ is set even if 'async_exec_thread_' is cancelling the query (which ensures that both threads call Coordinator::Cancel() and synchronize there). This is only done on the finalizing thread. Other threads that initiate cancellation should not block on cancellation, so that threads cannot pile up waiting for the query. Testing: Added a regression test that failed before this change. Ran exhaustive tests. Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M tests/util/cancel_util.py 5 files changed, 57 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/54/15954/5 -- To view, visit http://gerrit.cloudera.org:8080/15954 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I62cacc06d9877d33b79a33aeb3b82195e639b5c4 Gerrit-Change-Number: 15954 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong
