Tim Armstrong has posted comments on this change. Change subject: IMPALA-4409: respect lock order in QueryExecState::CancelInternal() ......................................................................
Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4896/1/be/src/service/query-exec-state.cc File be/src/service/query-exec-state.cc: PS1, Line 833: if (check_inflight) { : DCHECK(session_lock.owns_lock()); : if (session_->inflight_queries.find(query_id()) == : session_->inflight_queries.end()) { : return Status("Query not yet running"); : } > Are you aware of any reason we can't just do this check before taking lock_ It doesn't look like it would protect against anything - I did a pass through the code. Based on the history of the code, 'check_inflight' is meant to guard against the query being cancelled early when it is in a state with partially set-up data structures, so the race where it is removed from 'inflight_queries' during cancellation should be benign. -- To view, visit http://gerrit.cloudera.org:8080/4896 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I785fea0163a90d0633fb6ed77ec7c6882ab5c110 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
