Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/21452 )
Change subject: IMPALA-13088: (part 2) Parallelize final sorts in IcebergDeleteBuilder ...................................................................... Patch Set 2: (7 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/21452/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21452/2//COMMIT_MSG@33 PS2, Line 33: build fragments > aren't these 'probe' fragments that are blocked on the builder? Thanks for catching this, done. http://gerrit.cloudera.org:8080/#/c/21452/2//COMMIT_MSG@37 PS2, Line 37: The > nit: typo Done http://gerrit.cloudera.org:8080/#/c/21452/2//COMMIT_MSG@57 PS2, Line 57: lowered > reduced? Done http://gerrit.cloudera.org:8080/#/c/21452/2/be/src/exec/iceberg-delete-builder.h File be/src/exec/iceberg-delete-builder.h: http://gerrit.cloudera.org:8080/#/c/21452/2/be/src/exec/iceberg-delete-builder.h@147 PS2, Line 147: Status FinalizeBuildImplParallel(int num_threads); > Since this one can return error codes too, could you add a comment in what Done http://gerrit.cloudera.org:8080/#/c/21452/2/be/src/exec/iceberg-delete-builder.cc File be/src/exec/iceberg-delete-builder.cc: http://gerrit.cloudera.org:8080/#/c/21452/2/be/src/exec/iceberg-delete-builder.cc@314 PS2, Line 314: IcebergDeleteBuilder::DeleteRowVector IcebergDeleteBuilder::GetFinalDeleteRowVector( > This and the other PR made me think about whther it makes sense to create a That's a good idea. However, I have another Jira ticket to throw out all of these, or at least the final delete vectors, and use RoaringBitmap: IMPALA-13109 I'm afraid I'd have to throw out half of the work with that change. So probably it would be better to do the encapsulation and refactoring for the RoaringBitmap. http://gerrit.cloudera.org:8080/#/c/21452/2/be/src/exec/iceberg-delete-builder.cc@320 PS2, Line 320: int64_t capacity = 0; > indentation Done http://gerrit.cloudera.org:8080/#/c/21452/2/be/src/exec/join-builder.h File be/src/exec/join-builder.h: http://gerrit.cloudera.org:8080/#/c/21452/2/be/src/exec/join-builder.h@197 PS2, Line 197: int probes_waiting_on_builder_ = 0; > The variable name is self-explanatory, but some additional comment that is Done -- To view, visit http://gerrit.cloudera.org:8080/21452 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ca946a452d061238255e9b0e2c81a51cac68807 Gerrit-Change-Number: 21452 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 13 Jun 2024 16:59:31 +0000 Gerrit-HasComments: Yes
