Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/21138 )
Change subject: IMPALA-12809: Iceberg metadata table scanner should always be scheduled to the coordinator ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/21138/2/be/src/scheduling/schedule-state.h File be/src/scheduling/schedule-state.h: http://gerrit.cloudera.org:8080/#/c/21138/2/be/src/scheduling/schedule-state.h@107 PS2, Line 107: bool is_root_coord_fragment; now that we have 2 similar things 'is_root_coord_fragment' and 'fragment.must_run_on_coordinator' could you add a comment here to make more clear what this is for. http://gerrit.cloudera.org:8080/#/c/21138/2/common/thrift/Planner.thrift File common/thrift/Planner.thrift: http://gerrit.cloudera.org:8080/#/c/21138/2/common/thrift/Planner.thrift@51 PS2, Line 51: 15: required bool must_run_on_coordinator I know the order of the IDs is confusing now, but I feel this would be better at the end of this struct. http://gerrit.cloudera.org:8080/#/c/21138/2/fe/src/main/java/org/apache/impala/planner/PlanFragment.java File fe/src/main/java/org/apache/impala/planner/PlanFragment.java: http://gerrit.cloudera.org:8080/#/c/21138/2/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@93 PS2, Line 93: mustRunOnCoord_ nit: 'coordinatorOnly_' ? http://gerrit.cloudera.org:8080/#/c/21138/2/fe/src/main/java/org/apache/impala/planner/PlanFragment.java@193 PS2, Line 193: outputPartition_.equals(DataPartition.UNPARTITIONED)); 'outputPartition_' is unconditionally set to UNPARTITIONED in this function so this check doesn't make much sense. Also, I;m not sure we want to make this a constructor param. Can we default it to false and set it true in a setMustRunOnCoord() or such? We can then assert on the 'outputPartition_' there. Or did you mean 'dataPartition_'? -- To view, visit http://gerrit.cloudera.org:8080/21138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib4397f64e9def42d2b84ffd7bc14ff31df27d58e Gerrit-Change-Number: 21138 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Noemi Pap-Takacs <npaptak...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 21 Mar 2024 15:04:04 +0000 Gerrit-HasComments: Yes
