impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-5749: coordinator race hits DCHECK 'num remaining backends > 0'
Date Fri, 11 Aug 2017 19:23:05 GMT
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)
File be/src/runtime/

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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I1528661e5df6d9732ebfeb414576c82ec5c92241
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Michael Ho <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Reviewer: Thomas Tauber-Marshall <>
Gerrit-HasComments: Yes

View raw message