Drop velocity-based estimation in favor of driver path completion estimation#552
Drop velocity-based estimation in favor of driver path completion estimation#552gabotechs wants to merge 1 commit into
Conversation
c358e53 to
f53c5ba
Compare
f53c5ba to
1ed6245
Compare
1ed6245 to
64528a4
Compare
| /// truncated to this max value, as it's assumed that [READY_CHUNK_LIMIT] was hit and no useful | ||
| /// measurement can actually be emitted. | ||
| const MAX_ROWS_PER_SECOND: usize = 1024 * 1024; | ||
| const READY_CHUNK_LIMIT: usize = 1024 * 1024; |
There was a problem hiding this comment.
Increased this value on purpose. The idea is that we should be able to yield as much as possible here syncrhonously, as these are record batches that are anyways already in-memory, so we prefer to sample as much as possible, given that they are already there.
6c3a271 to
5da24a0
Compare
5da24a0 to
0ed7511
Compare
c191f38 to
96a4ec4
Compare
0ed7511 to
d605408
Compare
0c9855b to
ada6559
Compare
d605408 to
674fd1c
Compare
ada6559 to
53809fb
Compare
2ef9809 to
5bebc68
Compare
050416b to
aac68dd
Compare
c27c116 to
563d8a4
Compare
0a25d08 to
71c347f
Compare
0404839 to
f1db5c4
Compare
71c347f to
e7ee621
Compare
asolimando
left a comment
There was a problem hiding this comment.
LGTM, I have a few questions on the recursion (jump notation and chasing only the right child for joins), the rest is solid and I agree that removing the temporal notion from the equation (especially with an unknown total duration) will provide more stable estimates.
| // mapping from SamplerExec to leaf node. | ||
| // | ||
| // Just dividing by `partition_count` gives a good approximation, but it's still | ||
| // not perfect as it will not account for data skews. |
There was a problem hiding this comment.
This is a perfectly reasonable start, I agree.
As a future improvement, I wonder if we can apply sampling similarly to get an idea of the degree of data skew we have across partitions. wdyt?
| if Arc::ptr_eq(p, &child) { TreeNodeRecursion::Jump } else { TreeNodeRecursion::Continue } | ||
| }), | ||
| @r" | ||
| Single |
There was a problem hiding this comment.
Nit: how hard would it be to have an indentation level for children? it's not a big deal as I can read the plan declaration to see what are the children, but if not hard it would have better readability
| let rec = decide(p); | ||
| let suffix = match rec { | ||
| TreeNodeRecursion::Continue => "", | ||
| TreeNodeRecursion::Jump => " [->jump]", |
There was a problem hiding this comment.
Q: I am not sure I got the "jump" notation, what does this correspond to in a distributed plan? A recursive step to model DAGs?
| recurse(hash_join.right(), f) | ||
| } else if let Some(nested_loop_join) = plan.downcast_ref::<NestedLoopJoinExec>() { | ||
| recurse(nested_loop_join.right(), f) | ||
| } else if let Some(cross_join) = plan.downcast_ref::<CrossJoinExec>() { | ||
| recurse(cross_join.right(), f) |
There was a problem hiding this comment.
Q: why do we only follow the right children for these join nodes?
f1db5c4 to
f4096f2
Compare
e7ee621 to
82e6199
Compare
f4096f2 to
b6b6c77
Compare
1610648 to
94eec9c
Compare
b6b6c77 to
6bcf962
Compare
This PR enhances the benchmarks by comparing estimated bytes vs actual bytes in network boundaries during dynamic planninng: - Compare sampled bytes with actual network-boundary output bytes using q-error. - Store it per iteration in local and remote benchmarks. - Report median q-error (`QERR P50`) in benchmark comparisons. Preliminary step for #552. --- Sample output: ``` === Comparing tpch_sf10 results from branch 'stats-accuracy-benches' [prev] with 'stats-accuracy-benches' [new] === os: macos arch: aarch64 cpu cores: 16 threads: 2 -> 2 workers: 8 -> 8 =================================================================================================================== tpch_sf10 q1: prev= 327 ms, new= 355 ms, diff=1.09 slower ✖ tpch_sf10 q2: prev= 271 ms, new= 278 ms, diff=1.03 slower ✖ tpch_sf10 q3: prev= 378 ms, new= 346 ms, diff=1.09 faster ✔ tpch_sf10 q4: prev= 132 ms, new= 129 ms, diff=1.02 faster ✔ tpch_sf10 q5: prev= 617 ms, new= 610 ms, diff=1.01 faster ✔ tpch_sf10 q6: prev= 115 ms, new= 109 ms, diff=1.06 faster ✔ tpch_sf10 q7: prev= 765 ms, new= 743 ms, diff=1.03 faster ✔ tpch_sf10 q8: prev= 781 ms, new= 889 ms, diff=1.14 slower ✖ tpch_sf10 q9: prev=1040 ms, new=1010 ms, diff=1.03 faster ✔ tpch_sf10 q10: prev= 448 ms, new= 455 ms, diff=1.02 slower ✖ tpch_sf10 q11: prev= 132 ms, new= 121 ms, diff=1.09 faster ✔ tpch_sf10 q12: prev= 200 ms, new= 205 ms, diff=1.02 slower ✖ tpch_sf10 q13: prev= 233 ms, new= 232 ms, diff=1.00 faster ✔ tpch_sf10 q14: prev= 158 ms, new= 171 ms, diff=1.08 slower ✖ tpch_sf10 q15: prev= 262 ms, new= 241 ms, diff=1.09 faster ✔ tpch_sf10 q16: prev= 92 ms, new= 94 ms, diff=1.02 slower ✖ tpch_sf10 q17: prev= 833 ms, new= 816 ms, diff=1.02 faster ✔ tpch_sf10 q18: prev= 917 ms, new= 946 ms, diff=1.03 slower ✖ tpch_sf10 q19: prev= 187 ms, new= 199 ms, diff=1.06 slower ✖ tpch_sf10 q20: prev= 422 ms, new= 420 ms, diff=1.00 faster ✔ tpch_sf10 q21: prev=1044 ms, new=1022 ms, diff=1.02 faster ✔ tpch_sf10 q22: prev= 101 ms, new= 103 ms, diff=1.02 slower ✖ TOTAL: prev=9455 ms, new=9494 ms, diff=1.00 slower ✖ QERR P50: prev=n/a, new=19.47x ```
6bcf962 to
1d003f1
Compare
1d003f1 to
958a6f7
Compare
Summary
Replaces the velocity-based runtime-statistics estimate used by the dynamic task-count planner with a driver-path completion estimate.
The sampler on top of each stage feeds statistics (final row/byte counts) back to the coordinator, which uses them to size the stage above. The old estimate extrapolated those final sizes from a sampled throughput multiplied by a fixed, assumed query duration, which systematically overestimated — often by orders of magnitude. The new estimate instead scales what a stage has already produced by how far it is through its known total work.
How it worked before
The
SamplerExecreported, per partition:*_ready— rows / bytes already materialized at sampling time, and*_per_second— a sampled production velocity (rows/bytes per second).gather_runtime_statisticsextrapolated the final size as:i.e. "what's ready now, plus the sampled rate projected over 10 more seconds of execution."
While doing some benchmarks, I've seen that this approach heavily over-estimates row count.
How the new estimate avoids it
Instead of projecting a velocity over a made-up duration, it computes an actual completion fraction and divides by it:
rows_pulled_from_leaf— how many rows the stage has actually pulled from its leaf scans so far (now reported by the sampler in place of the velocity fields).estimated_driver_path_leaf_rows— the total number of rows the stage must pull to finish, obtained by summing the leafnum_rowsstatistics along the driver path: the probe-side chain that actually streams to the output. Join build sides are excluded because they are one-off setup work, not output progress (see the newapply_driver_pathhelper). These counts come from real source statistics (e.g. parquet row counts).Caveats
If a stage's leaves expose no row-count statistics, it falls back to a fixed
FALLBACK_ESTIMATED_PCT_SAMPLED = 0.2(assume ~20 % sampled).Benchmarks
TPCH-SF1: 1.02 faster
TPCH-SF10: 1.04 faster
TPCH-SF100: 1.01 faster
TPC-DS-SF1: 1.01 slower
ClickBench: 1.01 slower
Performance is not impact because both in
mainand this PR max out on tasks for a cluster of 12 workers.Regarding q-error on stats estimation, here are the results:
TPCH-SF1:
QERR P50: prev=1.80x, new=1.80x
QERR P95: prev=6.75x, new=2.29x
TPCH-SF10:
QERR P50: prev=1.80x, new=1.45x
QERR P95: prev=8.94x, new=2.12x
TPCH-SF100:
QERR P50: prev=2.12x, new=1.73x
QERR P95: prev=9.68x, new=2.16x
TPC-DS-SF1:
QERR P50: prev=1.54x, new=1.45x
QERR P95: prev=4.62x, new=4.01x
ClickBench:
QERR P50: prev=1.42x, new=1.45x
QERR P95: prev=1.42x, new=1.45x