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 8: (6 comments) http://gerrit.cloudera.org:8080/#/c/12299/7/be/src/runtime/coordinator-backend-state.cc File be/src/runtime/coordinator-backend-state.cc: http://gerrit.cloudera.org:8080/#/c/12299/7/be/src/runtime/coordinator-backend-state.cc@174 PS7, Line 174: last_report_time_ms_ = GenerateReportTimestamp(); : exec_complete_barrier->Notify(); > Does it implicitly rely on the LIFO order for local variables' destruction Done http://gerrit.cloudera.org:8080/#/c/12299/7/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/12299/7/be/src/runtime/coordinator.cc@549 PS7, Line 549: DCHECK_ > DCHECK_LE Done http://gerrit.cloudera.org:8080/#/c/12299/7/be/src/runtime/coordinator.cc@664 PS7, Line 664: DCHECK_ > DCHECK_LE Done http://gerrit.cloudera.org:8080/#/c/12299/4/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/12299/4/be/src/service/impala-server.cc@215 PS4, Line 215: DEFINE_int32(status_report_interval_ms, 5000, > right, I think I meant 0.1 * GetMaxReportRetryMs(). ie we need to check muc Done http://gerrit.cloudera.org:8080/#/c/12299/4/be/src/service/impala-server.cc@2288 PS4, Line 2288: int64_t cpu_limit_s = crs->query_options().cpu_limit_s; : int64_t cpu_limit_ns = cpu_limit_s * 1000'000'000L; : if (cpu_limit_ns > 0 && cpu_time_ns > cpu_limit_ns) { : Status err = Status::Expected(TErrorCode::CPU_LIMIT_EXCEEDED, : PrintId(crs->query_id()), PrettyPrinter::Print(cpu_limit_s, TUnit::TIME_S)); : VLOG_QUERY << err.msg().msg(); > The executor's reporting path currently can tolerate up to FLAGS_status_rep As discussed offline, I have a preference for the alternative option you list, as it minimizes the probability that a user can set the flags in a way that is fundamentally wrong while still leaving a lot of room to tune the behavior. http://gerrit.cloudera.org:8080/#/c/12299/7/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/12299/7/be/src/service/impala-server.cc@2258 PS7, Line 2258: 64_t lag_time_ms > per comment elsewhere: don't we need to wake up more often than the timeout 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: 8 Gerrit-Owner: Thomas Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Thomas Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Fri, 19 Apr 2019 23:38:26 +0000 Gerrit-HasComments: Yes
