Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21563 )
Change subject: IMPALA-13194: Fast-serialize position delete records ...................................................................... Patch Set 2: Code-Review+1 (4 comments) http://gerrit.cloudera.org:8080/#/c/21563/2/be/src/runtime/iceberg-position-delete-collector.h File be/src/runtime/iceberg-position-delete-collector.h: http://gerrit.cloudera.org:8080/#/c/21563/2/be/src/runtime/iceberg-position-delete-collector.h@30 PS2, Line 30: /// serializing them to outbound row batches. Can you add the layout from the commit message? Also, it could be mentioned what parts of memory are tracked and what are not. http://gerrit.cloudera.org:8080/#/c/21563/2/be/src/runtime/iceberg-position-delete-collector.h@63 PS2, Line 63: Reset This could be called inside Close and made private. http://gerrit.cloudera.org:8080/#/c/21563/2/be/src/runtime/outbound-row-batch.h File be/src/runtime/outbound-row-batch.h: http://gerrit.cloudera.org:8080/#/c/21563/2/be/src/runtime/outbound-row-batch.h@30 PS2, Line 30: KrpcDataStreamSender Is this really needed? http://gerrit.cloudera.org:8080/#/c/21563/2/be/src/runtime/outbound-row-batch.h@68 PS2, Line 68: Prepares Maybe PrepareForSend() would be clearer? It seem counter-intuitive for Finalize to be called more than once. -- To view, visit http://gerrit.cloudera.org:8080/21563 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6095f318e3d06dedb4197681156b40dd2a326c6f Gerrit-Change-Number: 21563 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Mon, 15 Jul 2024 13:07:39 +0000 Gerrit-HasComments: Yes
