Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/18314 )
Change subject: IMPALA-11176: Fix Memory leak in ClientCacheHelper ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/18314/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18314/2//COMMIT_MSG@11 PS2, Line 11: The memory leak happens because a shared pointer of the > nit: 'shared' Done http://gerrit.cloudera.org:8080/#/c/18314/2//COMMIT_MSG@18 PS2, Line 18: The way to fix in the patch is to make the destructor of : "ThriftClientImpl" virtual. Added testcase ClientCacheTest. : : Tests: : Ran exhaustive tests. : > We could probably remove this paragraph? Or maybe simply add "The fix is to Done http://gerrit.cloudera.org:8080/#/c/18314/2/be/src/runtime/client-cache-test.cc File be/src/runtime/client-cache-test.cc: http://gerrit.cloudera.org:8080/#/c/18314/2/be/src/runtime/client-cache-test.cc@70 PS2, Line 70: uint64_t GetProcessVMSize() { > Would have been ideal to get memory usage using memory pools and mem tracke Yes, the testcase is using the exact same way as how the memory was leaking in our system. Tried the test in the old code, and it couldn't pass. -- To view, visit http://gerrit.cloudera.org:8080/18314 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2bf3d7e7d897c89eb4538df5382ce3c590055b71 Gerrit-Change-Number: 18314 Gerrit-PatchSet: 3 Gerrit-Owner: Yida Wu <wydbaggio...@gmail.com> Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Yida Wu <wydbaggio...@gmail.com> Gerrit-Comment-Date: Mon, 14 Mar 2022 07:28:06 +0000 Gerrit-HasComments: Yes
