Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16215 )
Change subject: IMPALA-5746: Cancel all queries scheduled by failed coordinators ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/16215/3/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: http://gerrit.cloudera.org:8080/#/c/16215/3/be/src/runtime/exec-env.cc@554 PS3, Line 554: server->CancelQueriesOnFailedBackends(current_backend_set); I think this might be unnecessary on executor-only nodes? http://gerrit.cloudera.org:8080/#/c/16215/3/be/src/runtime/exec-env.cc@560 PS3, Line 560: cluster_membership_mgr_->RegisterUpdateCallbackFn( : [server](ClusterMembershipMgr::SnapshotPtr snapshot) { : std::unordered_set<BackendIdPB> current_backend_set; : for (const auto& it : snapshot->current_backends) { : current_backend_set.insert(it.second.backend_id()); : } : server->CancelQueriesOnFailedBackends(current_backend_set); : }); not necessarily related to your change, but it would be nice to make this callback specific to coordinator nodes, because I think CancelQueriesOnFailedBackends only ever does anything on coordinator nodes. http://gerrit.cloudera.org:8080/#/c/16215/3/be/src/runtime/query-exec-mgr.cc File be/src/runtime/query-exec-mgr.cc: http://gerrit.cloudera.org:8080/#/c/16215/3/be/src/runtime/query-exec-mgr.cc@222 PS3, Line 222: // TODO: create cancellation task queue and working thread to run cancellation tasks : // on a separate thread. If the queue is full, ignore the cancellations since we'll : // be able to process them on the next heartbeat instead. : : for (auto& qs : to_cancel) { : VLOG(1) << "CancelQueriesForFailedCoordinators(): cancel query " << qs->query_id(); : qs->Cancel(); : qs->is_coord_active_.Store(false); : ReleaseQueryState(qs); : } if you move this into impala-server.cc you can use the ImpalaServer::CancelFromThreadPool. see ImpalaServer::CancelQueriesOnFailedBackends -- To view, visit http://gerrit.cloudera.org:8080/16215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I918fcc27649d5d2bbe8b6ef47fbd9810ae5f57bd Gerrit-Change-Number: 16215 Gerrit-PatchSet: 3 Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Tue, 21 Jul 2020 18:44:55 +0000 Gerrit-HasComments: Yes
