Doris-Breakwater commented on issue #66282: URL: https://github.com/apache/doris/issues/66282#issuecomment-5128593618
Initial assessment: **valid, code-confirmed profile-generation bug**. The query itself can complete, but FE can throw while constructing the merged profile and lose the merged diagnostic data for the whole query. The same vulnerable `ExecutionProfile` loop is present on current `master` and `branch-4.0`. The issue currently has no labels; PR #66283 is open and targets `master`. ### Code-backed findings - `ExecutionProfile.getMultiBeProfile()` first computes the maximum `profileSingleBE.size()`, then unconditionally calls `profileSingleBE.get(pipelineIdx)` for every backend. If any backend list is shorter than that maximum, `IndexOutOfBoundsException` is deterministic; no additional log evidence is required to confirm this failure mode. - Uneven counts are a legitimate BE result, particularly on the branch-4.0 BE-side local-shuffle path. `PipelineFragmentContext::_add_local_exchange()` returns without adding a pipeline when that BE has `_num_instances <= 1`, but otherwise calls `add_pipeline(cur_pipe, pip_idx + 1)` (and can add another at `pip_idx + 2`). Thus two BEs executing the same fragment with different local instance counts can serialize different pipeline-list lengths. - Current `master` normally has an FE local-exchange-planning path, but the BE fallback remains reachable when local-shuffle planning is disabled, so the FE merger still must tolerate uneven topology. - PR #66283's bounds check is **necessary and correctly prevents the reported exception**. Its unit test also reproduces the direct short-list failure. There is, however, a second correctness concern that the current PR test does not cover: a bounds check alone assumes the common list indices still identify equivalent pipelines. That assumption does not hold for BE-side insertion. For example, starting from `[P0, P1]`, inserting a local-exchange pipeline after `P0` produces `[P0, LE, P1]` on one BE while another BE remains `[P0, P1]`. Positional merging then combines `LE` from the first BE with `P1` from the second at index 1; index 2 contains only the first BE's `P1`. The exception is gone, but operator counters and instance counts can be assigned to the wrong merged pipeline or omitted. `RuntimeProfile.mergeProfiles()` matching child names limits some cross-merging, but it does not restore the missing pipeline alignment. The new test in #66283 models a missing tail (`[P0, P1]` versus `[P0]`) and checks only the resulting tree shape. It does not model an inserted middle pipeline or assert that operator identities/counters from all BEs land in the correct aggregate. ### Information still useful The supplied report is sufficient to confirm the exception. To validate the real topology and design a correctness-complete merger, a sanitized occurrence should additionally provide: 1. Exact FE and BE commit/build identifiers, plus `enable_local_shuffle` and `enable_local_shuffle_planner`. 2. Per-BE fragment instance counts for the affected fragment. 3. The raw, unmerged per-BE pipeline profile trees (pipeline order and operator/node identities are the important parts). 4. The complete FE stack trace and a minimal SQL/DDL/data-distribution reproducer if one is available. These are not prerequisites for accepting the bounds check as crash containment, but they are needed to prove that the merged output remains semantically correct when the extra pipelines are inserted in the middle. ### Recommended next steps 1. Treat #66283 as the immediate crash-containment fix, but do not consider the overall uneven-topology merge fully solved by the bounds check alone. 2. Extend the unit coverage with profiles shaped like `[P0, P1]` and `[P0, LE, P1]`, using distinct operator node IDs/counters, and assert both that no exception occurs and that no `LE`/`P1` data are cross-merged. 3. For the complete fix, stop using the per-BE list position as the sole cross-BE identity. Either report and match a canonical pipeline topology/signature, or aggregate operator data by stable operator/node identity while representing backend-local exchange pipelines separately. A raw numeric pipeline ID should be validated carefully because splitting a pipeline changes its operator membership even when one side retains the old ID. 4. After master validation, backport the crash fix and the alignment-safe behavior to `branch-4.0`, since that branch contains the same FE bug and relies on the BE-side local-exchange path. 5. Add an appropriate `bug`/profile-area label and link #66283 explicitly from the issue metadata. Breakwater-GitHub-Analysis-Slot: slot_c8d79e709cbc -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
