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: (5 comments) http://gerrit.cloudera.org:8080/#/c/12299/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12299/1//COMMIT_MSG@29 PS1, Line 29: - Write functional tests once the appropriate mechanisms are in place > I think you can "kill -STOP" an impalad running a query, and it will end up That scenario is actually already handled prior to this patch by the statestore detecting the impalad is down. This patch is for queries where the impalad doesn't fail but status reports aren't getting sent for some reason. http://gerrit.cloudera.org:8080/#/c/12299/1/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/12299/1/be/src/runtime/coordinator.cc@213 PS1, Line 213: } > This could probably just use backend_states_.empty() instead. I don't mind Done http://gerrit.cloudera.org:8080/#/c/12299/1/be/src/runtime/coordinator.cc@744 PS1, Line 744: int64_t current_time = BackendState::GenerateReportTimestamp(); > It's confusing, as be/src/runtime/coordinator-backend-state.h has similarly Yeah, this is subtle. Added a helper function to make it clearer/reduce the chances we mess this up in the future. http://gerrit.cloudera.org:8080/#/c/12299/1/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/12299/1/be/src/service/impala-server.cc@2212 PS1, Line 2212: > Do we have a style to write Done http://gerrit.cloudera.org:8080/#/c/12299/1/be/src/service/impala-server.cc@2209 PS1, Line 2209: TNetworkAddress address; : int64_t lag_time_ms = coord->GetMaxBackendStateLagMs(&address); : if (lag_time_ms > FLAGS_max_report_lag_s * 1000) { : cancellation_thread_pool_->Offer( > If I were a user receiving this error, I'd immediately want to know which b 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 <thomasmarsh...@cmu.edu> 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 <thomasmarsh...@cmu.edu> Gerrit-Comment-Date: Wed, 30 Jan 2019 21:59:16 +0000 Gerrit-HasComments: Yes
