Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/16911 )
Change subject: IMPALA-10413: fix impalad crashes when canceling the retrying query ...................................................................... Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/16911/8/be/src/runtime/query-driver.cc File be/src/runtime/query-driver.cc: http://gerrit.cloudera.org:8080/#/c/16911/8/be/src/runtime/query-driver.cc@273 PS8, Line 273: if not call Finalize, the wait-thread will leak Maybe I'm misunderstanding something. ClientRequestState::Finalize() just blocks until the wait-thread exits. It does nothing to terminate the wait-thread. Why calling Finalize() will help to prevent leaks? The blocking point of the wait-thread is ClientRequestState::WaitInternal(). For a SELECT statement, it should exit after the coordinator fragment instance finishes/fails Prepare() and Open(). See more in FragmentInstanceState::Exec() about how opened_promise_ is set. Usually, ClientRequestState::WaitInternal() is waiting in Coordinator::Wait() which will wait in coord_instance_->WaitForOpen(). WaitForOpen() should return when the original query is cancelled. If the wait-thread is still waiting, maybe there are something wrong in cancelling the coordinator fragment instance thread. We need to check deeper in that point. Do you have a way to reproduce the leaking? http://gerrit.cloudera.org:8080/#/c/16911/8/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/16911/8/be/src/service/impala-server.cc@2248 PS8, Line 2248: retried_query_id = make_unique<TUniqueId>(query_handle.retried_id()); Any reason for not making was_retried = query_handle.WasRetried() && query_handle.IsSetRetriedId()? The current code will introduce a new state that was_retried is true but the retried_query_id is nullptr. This will break some DCHECKs: https://github.com/apache/impala/blob/4099a606892c377b9e8c9c6df2a45a7d42afcaea/be/src/service/impala-server.cc#L897 https://github.com/apache/impala/blob/4099a606892c377b9e8c9c6df2a45a7d42afcaea/be/src/service/impala-server.cc#L739 -- To view, visit http://gerrit.cloudera.org:8080/16911 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4fd7228acd0a70d33859029052239f9b9f795e5d Gerrit-Change-Number: 16911 Gerrit-PatchSet: 8 Gerrit-Owner: Xianqing He <hexianqing...@126.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: Xianqing He <hexianqing...@126.com> Gerrit-Comment-Date: Tue, 05 Jan 2021 08:35:14 +0000 Gerrit-HasComments: Yes
