[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Quanlong Huang has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. IMPALA-10414: fix memory leak when canceling the retried query The query retry launches in a separate thread. This thread may not finishes when deleting the query from the given QueryDriverMap if the query retry was failed launched. In this case, the resources for the query retry thread will not release. So the reference count of QueryDriver (via the shared_ptr) will not go to 0 and it will not be destroyed. We need wait until the query retry thread execution has completed when deleting the query from the given QueryDiverMap. Testing: Modify the test_query_retries.py to verify memory leak by checking the debug web UI of memz. Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Reviewed-on: http://gerrit.cloudera.org:8080/17735 Reviewed-by: Wenzhe Zhou Tested-by: Impala Public Jenkins Reviewed-by: Quanlong Huang --- M be/src/runtime/query-driver.cc M be/src/service/client-request-state.cc M tests/custom_cluster/test_query_retries.py 3 files changed, 37 insertions(+), 2 deletions(-) Approvals: Wenzhe Zhou: Looks good to me, but someone else must approve Impala Public Jenkins: Verified Quanlong Huang: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 11 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 10: Code-Review+2 Thanks for your review, Wenzhe! and thank Xianqing's hard work! -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 10 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Fri, 15 Apr 2022 01:36:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 10: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 10 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Thu, 14 Apr 2022 21:07:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 10: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8048/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 10 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Thu, 14 Apr 2022 16:39:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 10: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 10 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Thu, 14 Apr 2022 16:38:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/10451/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 10 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Thu, 14 Apr 2022 11:18:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Xianqing He has posted comments on this change. (
http://gerrit.cloudera.org:8080/17735 )
Change subject: IMPALA-10414: fix memory leak when canceling the retried query
..
Patch Set 10:
> Patch Set 9:
>
> Ran debug exhaustive test and asan test for this patch, plus following minor
> changes for ClientRequestState::Cancel()
>
> -admission_control_client_->CancelAdmission();
> -is_cancelled_ = true;
> +if (!is_cancelled_) {
> + admission_control_client_->CancelAdmission();
> + is_cancelled_ = true;
> +}
>} // Release lock_ before doing cancellation work.
>
> Passed debug exhaustive test without error
> (https://jenkins.impala.io/job/pre-review-test/1323/)
> Got one failure (TestAcid.test_lock_timings) for asan test.
> (https://master-03.jenkins.cloudera.com/job/impala-private-parameterized/915/).
> I think this failure is unrelavent.
Thanks for testing it. I had fixed as this.
--
To view, visit http://gerrit.cloudera.org:8080/17735
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f
Gerrit-Change-Number: 17735
Gerrit-PatchSet: 10
Gerrit-Owner: Xianqing He
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sahil Takiar
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Xianqing He
Gerrit-Comment-Date: Thu, 14 Apr 2022 11:00:48 +
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Xianqing He has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. IMPALA-10414: fix memory leak when canceling the retried query The query retry launches in a separate thread. This thread may not finishes when deleting the query from the given QueryDriverMap if the query retry was failed launched. In this case, the resources for the query retry thread will not release. So the reference count of QueryDriver (via the shared_ptr) will not go to 0 and it will not be destroyed. We need wait until the query retry thread execution has completed when deleting the query from the given QueryDiverMap. Testing: Modify the test_query_retries.py to verify memory leak by checking the debug web UI of memz. Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f --- M be/src/runtime/query-driver.cc M be/src/service/client-request-state.cc M tests/custom_cluster/test_query_retries.py 3 files changed, 37 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/17735/10 -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 10 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Wenzhe Zhou has posted comments on this change. (
http://gerrit.cloudera.org:8080/17735 )
Change subject: IMPALA-10414: fix memory leak when canceling the retried query
..
Patch Set 9:
Ran debug exhaustive test and asan test for this patch, plus following minor
changes for ClientRequestState::Cancel()
-admission_control_client_->CancelAdmission();
-is_cancelled_ = true;
+if (!is_cancelled_) {
+ admission_control_client_->CancelAdmission();
+ is_cancelled_ = true;
+}
} // Release lock_ before doing cancellation work.
Passed debug exhaustive test without error
(https://jenkins.impala.io/job/pre-review-test/1323/)
Got one failure (TestAcid.test_lock_timings) for asan test.
(https://master-03.jenkins.cloudera.com/job/impala-private-parameterized/915/).
I think this failure is unrelavent.
--
To view, visit http://gerrit.cloudera.org:8080/17735
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f
Gerrit-Change-Number: 17735
Gerrit-PatchSet: 9
Gerrit-Owner: Xianqing He
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sahil Takiar
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Xianqing He
Gerrit-Comment-Date: Wed, 13 Apr 2022 22:41:56 +
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc File be/src/runtime/query-driver.cc: http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc@368 PS8, Line 368: RETURN_VOID_IF_ERROR(query_handle->Finalize(true, nullptr)); > We can add a new API in Coordinator to wait until finalized. In ClientReque Read cancellation functions. ChildQuery::Cancel() and Coordinator::Cancel() are safe to be recalled. But RemoteAdmissionControlClient::CancelAdmission() could send extra RPC if it's recalled. We can make simple change for ClientRequestState::Cancel(), don't call admission_control_client_->CancelAdmission() if is_cancelled_ is already true. -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 9 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Wed, 13 Apr 2022 06:58:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc File be/src/runtime/query-driver.cc: http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc@368 PS8, Line 368: RETURN_VOID_IF_ERROR(query_handle->Finalize(true, nullptr)); > Agree. When ClientRequestState::Cancel() is called with wait_until_finalize We can add a new API in Coordinator to wait until finalized. In ClientRequestState::Cancel(), we can call this new API instead of Coordinator::Cancel() if both parameter wait_until_finalized and is_cancelled_ are true. -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 9 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Tue, 12 Apr 2022 05:52:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc File be/src/runtime/query-driver.cc: http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc@368 PS8, Line 368: RETURN_VOID_IF_ERROR(query_handle->Finalize(true, nullptr)); > Different places call ClientRequestState::Cancel() with different parameter Agree. When ClientRequestState::Cancel() is called with wait_until_finalized as true, we have to wait until finalized. Query retry make the cancellation code messy. I recently saw a case for which Coordinator::BackendState::Cancel() was hung and it caused admission resource not been released. I think it's not safe to call ClientRequestState::Cancel() more than once since it call AdmissionControlService::CancelAdmission() and Coordinator::Cancel(). -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 9 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Mon, 11 Apr 2022 05:24:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Xianqing He has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc File be/src/runtime/query-driver.cc: http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc@368 PS8, Line 368: RETURN_VOID_IF_ERROR(query_handle->Finalize(true, nullptr)); > Without calling ClientRequestState::Finalize(), the transaction, which is o Different places call ClientRequestState::Cancel() with different parameters, I'm not sure whether we will hit some illegal states when checking is_cancelled_ in the beginning of ClientRequestState::Cancel(). -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 9 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Mon, 11 Apr 2022 03:59:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Xianqing He has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 8: > Patch Set 8: > > > Patch Set 8: Code-Review+1 > > > > So sorry to be late here.. I triggered two exhaustive builds: > > https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/16239/ (DEBUG) > > https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/16240/ (ASAN) > > > > I think we can merge this after they pass. > > Hi Xianqing, > The two jobs failed. Could you have a look on them? I haven't digged into > them. Not sure if they are unrelated failures. I think they are unrelated failures and need to rebase the code. The failures were fixed in IMPALA-10961, IMPALA-11196 and IMPALA-11109. -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 8 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Mon, 11 Apr 2022 02:56:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 8: > Patch Set 8: Code-Review+1 > > So sorry to be late here.. I triggered two exhaustive builds: > https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/16239/ (DEBUG) > https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/16240/ (ASAN) > > I think we can merge this after they pass. Hi Xianqing, The two jobs failed. Could you have a look on them? I haven't digged into them. Not sure if they are unrelated failures. -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 8 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Sat, 09 Apr 2022 13:38:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc File be/src/runtime/query-driver.cc: http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc@368 PS8, Line 368: RETURN_VOID_IF_ERROR(query_handle->Finalize(true, nullptr)); > Thanks for catching this issue. This Finalize() call seems missing. One que Without calling ClientRequestState::Finalize(), the transaction, which is opened by original query, will not be closed. This is another way to validate ClientRequestState::Finalize() is missing. -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 8 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Sat, 09 Apr 2022 03:18:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc File be/src/runtime/query-driver.cc: http://gerrit.cloudera.org:8080/#/c/17735/8/be/src/runtime/query-driver.cc@368 PS8, Line 368: RETURN_VOID_IF_ERROR(query_handle->Finalize(true, nullptr)); Thanks for catching this issue. This Finalize() call seems missing. One question. ClientRequestState::Finalize() will call ClientRequestState::Cancel() for request_state again since the Cancel() function is already called in line 241 in this function. But ClientRequestState::Cancel() does not check if member variable is_cancelled_ is already true. I think ClientRequestState::Finalize() should check is_cancelled_ before call Cancel(), or ClientRequestState::Cancel() check is_cancelled_ in the beginning to avoid calling cancellation for coordinator and admission control again. -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 8 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Sat, 09 Apr 2022 01:27:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 8: Code-Review+1 So sorry to be late here.. I triggered two exhaustive builds: https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/16239/ (DEBUG) https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/16240/ (ASAN) I think we can merge this after they pass. -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 8 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Fri, 08 Apr 2022 06:50:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 8: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 8 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Fri, 11 Feb 2022 17:00:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/7834/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 8 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Fri, 11 Feb 2022 10:29:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Xianqing He has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/17735/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17735/5//COMMIT_MSG@7 PS5, Line 7: cancel > nit: cancelling Done http://gerrit.cloudera.org:8080/#/c/17735/5//COMMIT_MSG@11 PS5, Line 11: n this case, the resourc > Could you add more explanation about this? Done http://gerrit.cloudera.org:8080/#/c/17735/5/be/src/runtime/query-driver.cc File be/src/runtime/query-driver.cc: http://gerrit.cloudera.org:8080/#/c/17735/5/be/src/runtime/query-driver.cc@357 PS5, Line 357: request_state->MarkAsRetried(retry_query_id); > Do we only need this in the corner case or we need it in all cases? Could y Done http://gerrit.cloudera.org:8080/#/c/17735/5/be/src/runtime/query-driver.cc@419 PS5, Line 419: ueryHandle* query_handle, bool ch > nit: Wait until retry_query_thread_ finishes Done -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 7 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Xianqing He Gerrit-Comment-Date: Thu, 14 Oct 2021 08:47:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/9606/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 7 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 14 Oct 2021 08:47:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10414: fix memory leak when canceling the retried query
Xianqing He has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/17735 ) Change subject: IMPALA-10414: fix memory leak when canceling the retried query .. IMPALA-10414: fix memory leak when canceling the retried query The query retry launches in a separate thread. This thread may not finishes when deleting the query from the given QueryDriverMap if the query retry was failed launched. In this case, the resources for the query retry thread will not release. So the reference count of QueryDriver (via the shared_ptr) will not go to 0 and it will not be destroyed. We need wait until the query retry thread execution has completed when deleting the query from the given QueryDiverMap. Testing: Modify the test_query_retries.py to verify memory leak by checking the debug web UI of memz. Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f --- M be/src/runtime/query-driver.cc M tests/custom_cluster/test_query_retries.py 2 files changed, 31 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/17735/7 -- To view, visit http://gerrit.cloudera.org:8080/17735 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If804ca65da1794c819a6b2e6567ea7651ab5112f Gerrit-Change-Number: 17735 Gerrit-PatchSet: 7 Gerrit-Owner: Xianqing He Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
