Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/12299 )
Change subject: IMPALA-2990: timeout unresponsive queries in coordinator ...................................................................... Patch Set 2: (4 comments) > Any update ? Got distracted by stress test stuff, but yes I'll be updating this and my other pending krpc patch soon http://gerrit.cloudera.org:8080/#/c/12299/2/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/12299/2/be/src/runtime/coordinator-backend-state.h@137 PS2, Line 137: boost::lock_guard<boost::mutex> l(lock_); > In theory, this doesn't need any locking as read / write of a 64-bit word s Is that something we can rely on in all possible environments in which Impala might run? http://gerrit.cloudera.org:8080/#/c/12299/2/be/src/runtime/coordinator-backend-state.h@153 PS2, Line 153: /// Returns a timestamp for use in tracking arrival of status reports. > May help to state explicitly that this uses the monotonic time instead of U Done http://gerrit.cloudera.org:8080/#/c/12299/2/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/12299/2/be/src/runtime/coordinator-backend-state.cc@89 PS2, Line 89: last_report_time_ms_ = GenerateReportTimestamp(); > Should this clock not be started until we have issued all the initial ExecF Done http://gerrit.cloudera.org:8080/#/c/12299/2/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/12299/2/be/src/runtime/coordinator.cc@747 PS2, Line 747: for (BackendState* backend_state : backend_states_) { > Is there any race with the init code which populates > backend_states_ ? Shouldn't the detection be skipped until the > backend_states_ be fully populated ? This is handled by the use of 'backend_states_init_lock' above. Since we're moving to setting 'last_report_time' in Exec() instead of Init(), the new patch uses exec_rpcs_complete_barrier_ instead > > Does this handle the case in which a backend is done so no more new > report will be received ? Done -- To view, visit http://gerrit.cloudera.org:8080/12299 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I196c8c6a5633b1960e2c3a3884777be9b3824987 Gerrit-Change-Number: 12299 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Marshall <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Comment-Date: Tue, 26 Feb 2019 00:17:54 +0000 Gerrit-HasComments: Yes
