Henry Robinson has posted comments on this change. Change subject: IMPALA-5749: coordinator race hits DCHECK 'num_remaining_backends_ > 0' ......................................................................
Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/7577/2/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: PS2, Line 947: bool done; : if (!backend_state->ApplyExecStatusReport(params, &exec_summary_, &progress_, &done)) { : // ignore stray exec reports if we're already done, otherwise we lose : // track of num_remaining_backends_ We've got two 'done' concepts here that are close enough to make this a bit confusing. The first is whether the backend was already done *before* this report (return value), the second is whether the backend is done *after* this report is applied (output param). It seems to me that we could remove the output parameter. The idea is to have ApplyExecStatusReport() return true iff this is the report that switched the BE state to 'done'. Then I think we can clean the logic here up a bit: 1. UpdateInsertExecStatus() 2. if (ApplyExecStatusReport()) { // we are report that finished this backend state 2a UpdateStatus() // Only need to do if AESR() == true, otherwise we are not first bad status that this BE has reported 2b. --num_remaining_backends etc. 3 } else { if (returned_all_results_) return Status::Cancelled(); } 4 return Status::OK(); -- To view, visit http://gerrit.cloudera.org:8080/7577 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1528661e5df6d9732ebfeb414576c82ec5c92241 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
