Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/17869 )
Change subject: IMPALA-10956: datasketches UDFs: memory leak and merge overhead ...................................................................... Patch Set 3: Code-Review+2 (2 comments) Went through the code again, lgtm gave +2 - please write once again if you want the code to be merged as it is or add updates, and then I will start the tests http://gerrit.cloudera.org:8080/#/c/17869/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17869/3//COMMIT_MSG@9 PS3, Line 9: - call destructors of sketch and union objects >It did not seem to me that these comments invite a discussion or something >might be expected from me. The only actionable suggestion was adding tests with group by to have much more sketches, but it is not critical to do it in this change. > I believe there is a ticket open for the dependency issue. The one I know about is IMPALA-10868 http://gerrit.cloudera.org:8080/#/c/17869/3/be/src/exprs/aggregate-functions-ir.cc File be/src/exprs/aggregate-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/17869/3/be/src/exprs/aggregate-functions-ir.cc@1689 PS3, Line 1689: agg_state_ptr->second = new (ctx->Allocate<datasketches::hll_sketch>()) : datasketches::hll_sketch(DS_SKETCH_CONFIG, DS_HLL_TYPE); > As I understand, there is always the update phase of the processing before I agree that it is a great improvement as it is. If there is no group by, we do aggregation in two phases, one using update, then we serialize it, and send it to a final node that will merge the aggregated results from different nodes. -- To view, visit http://gerrit.cloudera.org:8080/17869 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8dd0e6736f4266f74f5f265f58d40a4e4707287f Gerrit-Change-Number: 17869 Gerrit-PatchSet: 3 Gerrit-Owner: Alexander Saydakov <[email protected]> Gerrit-Reviewer: Alexander Saydakov <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fucun Chu <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Wed, 08 Dec 2021 20:08:31 +0000 Gerrit-HasComments: Yes
