andygrove commented on issue #2024:
URL: 
https://github.com/apache/datafusion-ballista/issues/2024#issuecomment-5004188223

   Re-measured on `main` @ `26b29391`, same cluster shape as the original 
report (2 executors x 8 cores / 56 GiB, SF1000 on node-local disk, 
`target_partitions=32`, `prefer_hash_join=false`, dynamic-filter pushdown off, 
1 iteration, full-suite runs).
   
   **The regression reproduces, but the stated cause does not hold.**
   
   | Query | AQE off | AQE on | Delta | Originally reported |
   |-------|--------:|-------:|------:|--------------------:|
   | 3  | 206.2 | 255.0 | +24% | +31% |
   | 5  | 542.7 | 700.5 | +29% | +36% |
   | 7  | 575.5 | 531.4 | **−8%** | +13% |
   | 8  | 657.1 | 801.2 | +22% | +13% |
   | 9  | 891.8 | 1025.2 | +15% | +10% |
   | 17 | 697.8 | 400.4 | −43% | −48% |
   | 18 | 620.7 | 820.9 | +32% | — |
   | 19 | 39.6 | 42.1 | **+6%** | +188% |
   | **Total** | **6378.2** | **6320.7** | **−0.9%** | |
   
   Two specifics from the original report have changed: **all 22 queries now 
complete under the adaptive planner** (it no longer costs two queries), and 
**Q19's +188% is gone**. Q7 has flipped from a regression to an improvement. 
Q3/Q5/Q8/Q9/Q18 still regress, so the core complaint stands.
   
   Note the totals are a wash (−0.9%, inside this cluster's noise): the 
adaptive planner's wins on Q17/Q2/Q1 roughly cancel its losses on the 
join-heavy queries. The aggregate hides movement in both directions rather than 
showing nothing happening.
   
   **On the hypothesis.** The description reasons that the regressions are 
"what you would expect if the machinery is adding runtime overhead **without 
actually firing the broadcast promotion**". That is not what happens — the 
promotion does fire. Running the scheduler with 
`RUST_LOG=info,ballista_scheduler::state::aqe=debug` prints every resolver 
decision; Q2 at SF1000 gives:
   
   ```
   to_actual_join - plan_id: 1, decision: CollectLeft left: (Inexact(799140) | 
Absent), right: (Exact(10000000) | Absent)
   to_actual_join - plan_id: 2, decision: CollectLeft left: (Inexact(3196560) | 
Absent), right: (Exact(25) | Absent)
   to_actual_join - plan_id: 3, decision: CollectLeft left: (Inexact(3196560) | 
Absent), right: (Inexact(1) | Absent)
   to_actual_join - plan_id: 4, decision: Partitioned left: (Exact(800000000) | 
Exact(25628311552)), right: (Exact(10000000) | Exact(160374784))
   to_actual_join - plan_id: 5, decision: CollectLeft left: (Inexact(10000000) 
| Inexact(320000000)), right: (Exact(25) | Exact(400))
   to_actual_join - plan_id: 7, decision: CollectLeft left: (Inexact(639312) | 
Absent), right: (Inexact(159969600) | Inexact(3839270400))
   ```
   
   Six of ten decisions are `CollectLeft`. Broadcast promotion is working; this 
is not dead machinery.
   
   `hash_join_single_partition_threshold` is also no longer zero — #1900 set it 
to 10 MB — so the older "both broadcast mechanisms are effectively dead" 
reading is out of date.
   
   **What the decisions do show** is a different defect, now filed as #2081: 
when `total_byte_size` is `Absent`, the resolver falls back to comparing a row 
count against `hash_join_single_partition_threshold_rows` (1,000,000), which is 
size-blind. `plan_id: 1` and `plan_id: 7` above are broadcast at 799,140 and 
639,312 rows of *unknown* size — `part`/`supplier`-derived rows carrying string 
columns. Absent sizes are the norm here, since DataFusion discards 
`total_byte_size` on every join and one `Utf8` column stops 
`Statistics::calculate_total_byte_size` rebuilding it.
   
   Worth being clear that this does **not** yet explain the regressions. #2084 
makes the decision size-aware, and while it demonstrably changes the plan (Q2's 
799k-row broadcast becomes `Partitioned` while `nation`/`region` still 
broadcast), it produced **no measured speedup** at SF1000 — the one query whose 
plan it changes got marginally slower. So the cause of the Q3/Q5/Q8/Q9/Q18 
regressions is still open. Since broadcast promotion is firing, the remaining 
candidates are the cost of the promotions themselves, the extra shuffles from 
the `Unknown` -> `Repartition` -> re-decide path, or replanning overhead — not 
an absence of promotion.
   
   Method and current numbers: 
https://datafusion.apache.org/ballista/contributors-guide/benchmarking.html
   


-- 
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]

Reply via email to