You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2021/08/06 09:58:16 UTC

[Impala-ASF-CR] IMPALA-10414: fix memory leak when cancel 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 cancel the retried query
......................................................................


Patch Set 5:

(5 comments)

Thanks for uploading the fix! I'm still revisiting the cancellation and unregisteration codes. Post some questions for discussion first.

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


http://gerrit.cloudera.org:8080/#/c/17735/5//COMMIT_MSG@11
PS5, Line 11: and the memory will leak
Could you add more explanation about this?


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:   RETURN_VOID_IF_ERROR(query_handle->Finalize(true, nullptr));
Do we only need this in the corner case or we need it in all cases? Could you add a comment about this?


http://gerrit.cloudera.org:8080/#/c/17735/5/be/src/runtime/query-driver.cc@419
PS5, Line 419: Wait retry_query_thread_ finished
nit: Wait until retry_query_thread_ finishes


http://gerrit.cloudera.org:8080/#/c/17735/5/tests/custom_cluster/test_query_retries.py
File tests/custom_cluster/test_query_retries.py:

http://gerrit.cloudera.org:8080/#/c/17735/5/tests/custom_cluster/test_query_retries.py@1098
PS5, Line 1098: __validate_memz
Can we validate this at the end of all 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: 5
Gerrit-Owner: Xianqing He <he...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 06 Aug 2021 09:58:16 +0000
Gerrit-HasComments: Yes