impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5749: coordinator race hits DCHECK 'num remaining backends > 0'
Date Thu, 17 Aug 2017 17:37:12 GMT
Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-5749: coordinator race hits DCHECK 'num_remaining_backends_ > 0'
......................................................................


Patch Set 4:

(4 comments)

> (3 comments)
 > 
 > Thanks - this seems a lot better to me. Did you re-run
 > test_finst_cancel?

Yes, I ran it in a loop again.

http://gerrit.cloudera.org:8080/#/c/7577/2/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

PS2, Line 947: 
             :     // for now, abort the query if we see any error except if returned_all_results_
is
             :     // true (UpdateStatus() initiates cancellation, if it hasn't already been)
             :     // TODO: clarify control flow here,
> We've got two 'done' concepts here that are close enough to make this a bit
Done


http://gerrit.cloudera.org:8080/#/c/7577/3/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

PS3, Line 971:                  << backend_state->impalad_address();
             :           break;
             :         }
             :       }
> not your change, but while we're here: can you hoist the VLOG_QUERY_IS_ON c
Done


Line 987: 
> might be clearer just to return Status::OK() here, and avoid the need for a
Done


PS3, Line 991: nsert_exec_status.per_pa
> let's remove this. I can see the race where IsDone() becomes true after lin
I filed IMPALA-5807


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message