Respect memory pool size when in GroupedHashAggregateStream when spilling is not possible

[pepijnve]

Which issue does this PR close?

• Closes #19286.
• Related to #13123

Rationale for this change

GroupedHashAggregateStream currently always reports that it can spill to the memory tracking subsystem even though this is dependent on the aggregation mode and the grouping order. The optimistic logic in group_aggregate_batch also does not correctly take these conditions into account

What changes are included in this PR?

• Correctly set MemoryConsumer::can_spill to reflect actual spilling behaviour
• Align behaviour of group_aggregate_batch and spill_previous_if_necessary

Are these changes tested?

Added additional test case to demonstrate problem. This may not actually be necessary since other tests started failing as well. Still working on correcting those.

Are there any user-facing changes?

Yes, memory exhaustion may be reported much earlier in the query pipeline than is currently the case. In my local tests with a per consumer memory limit of 32MiB, grouped aggregation would consume 480MiB in practice. This was then reported by ExternalSortExec which choked on trying to reserve that much memory.

[pepijnve]

~wasm test failure seems unrelated.~ seems to have been a fluke

[pepijnve]

Extended test failures are related to the changes in this PR. Will review as soon as I can.

[pepijnve]

I've extended this PR to include spill support for grouping by partial or fully sorted inputs. This fixed the previously failing tests.

[alamb]

run benchmarks

[alamb]

run benchmark external_aggr

[alamb-ghbot]

🤖 ./gh_compare_branch.sh gh_compare_branch.sh Running Linux aal-dev 6.14.0-1018-gcp #19~24.04.1-Ubuntu SMP Wed Sep 24 23:23:09 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Comparing issue_19286 (d9af260747d28f0188ec7c571c68c3b36d988182) to e914935660875bc415887ade3fea1f5e59bed5e1 diff using: tpch_mem clickbench_partitioned clickbench_extended Results will be posted here when complete

[alamb-ghbot]

🤖: Benchmark completed

Details

Comparing HEAD and issue_19286
--------------------
Benchmark clickbench_extended.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Query        ┃        HEAD ┃ issue_19286 ┃    Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ QQuery 0     │  2768.36 ms │  2781.27 ms │ no change │
│ QQuery 1     │  1210.78 ms │  1206.99 ms │ no change │
│ QQuery 2     │  2327.35 ms │  2416.15 ms │ no change │
│ QQuery 3     │  1183.49 ms │  1180.56 ms │ no change │
│ QQuery 4     │  2325.06 ms │  2298.98 ms │ no change │
│ QQuery 5     │ 29417.81 ms │ 28624.36 ms │ no change │
│ QQuery 6     │  4148.20 ms │  4091.45 ms │ no change │
│ QQuery 7     │  3650.20 ms │  3630.43 ms │ no change │
└──────────────┴─────────────┴─────────────┴───────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary          ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)          │ 47031.25ms │
│ Total Time (issue_19286)   │ 46230.18ms │
│ Average Time (HEAD)        │  5878.91ms │
│ Average Time (issue_19286) │  5778.77ms │
│ Queries Faster             │          0 │
│ Queries Slower             │          0 │
│ Queries with No Change     │          8 │
│ Queries with Failure       │          0 │
└────────────────────────────┴────────────┘
--------------------
Benchmark clickbench_partitioned.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃        HEAD ┃ issue_19286 ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0     │     2.72 ms │     2.58 ms │ +1.06x faster │
│ QQuery 1     │    50.10 ms │    51.38 ms │     no change │
│ QQuery 2     │   138.01 ms │   136.19 ms │     no change │
│ QQuery 3     │   157.51 ms │   160.57 ms │     no change │
│ QQuery 4     │  1098.11 ms │  1119.95 ms │     no change │
│ QQuery 5     │  1500.24 ms │  1539.26 ms │     no change │
│ QQuery 6     │     2.32 ms │     2.31 ms │     no change │
│ QQuery 7     │    56.13 ms │    55.90 ms │     no change │
│ QQuery 8     │  1427.96 ms │  1490.07 ms │     no change │
│ QQuery 9     │  1875.95 ms │  1981.70 ms │  1.06x slower │
│ QQuery 10    │   378.53 ms │   362.97 ms │     no change │
│ QQuery 11    │   410.78 ms │   411.68 ms │     no change │
│ QQuery 12    │  1344.05 ms │  1409.03 ms │     no change │
│ QQuery 13    │  2038.46 ms │  2032.92 ms │     no change │
│ QQuery 14    │  1263.64 ms │  1304.08 ms │     no change │
│ QQuery 15    │  1257.14 ms │  1228.76 ms │     no change │
│ QQuery 16    │  2647.13 ms │  2698.07 ms │     no change │
│ QQuery 17    │  2641.55 ms │  2717.83 ms │     no change │
│ QQuery 18    │  4998.71 ms │  4972.88 ms │     no change │
│ QQuery 19    │   124.97 ms │   125.49 ms │     no change │
│ QQuery 20    │  1930.71 ms │  1928.07 ms │     no change │
│ QQuery 21    │  2261.38 ms │  2211.14 ms │     no change │
│ QQuery 22    │  3892.42 ms │  3826.63 ms │     no change │
│ QQuery 23    │ 12536.42 ms │ 12495.60 ms │     no change │
│ QQuery 24    │   219.14 ms │   205.82 ms │ +1.06x faster │
│ QQuery 25    │   460.07 ms │   462.27 ms │     no change │
│ QQuery 26    │   225.23 ms │   220.45 ms │     no change │
│ QQuery 27    │  2815.01 ms │  2776.16 ms │     no change │
│ QQuery 28    │ 24048.63 ms │ 23714.54 ms │     no change │
│ QQuery 29    │  1009.00 ms │  1006.84 ms │     no change │
│ QQuery 30    │  1343.16 ms │  1329.69 ms │     no change │
│ QQuery 31    │  1379.58 ms │  1335.89 ms │     no change │
│ QQuery 32    │  4738.56 ms │  4639.03 ms │     no change │
│ QQuery 33    │  5915.69 ms │  6034.39 ms │     no change │
│ QQuery 34    │  6102.79 ms │  6002.09 ms │     no change │
│ QQuery 35    │  2032.87 ms │  1890.81 ms │ +1.08x faster │
│ QQuery 36    │   120.08 ms │   119.18 ms │     no change │
│ QQuery 37    │    56.33 ms │    57.36 ms │     no change │
│ QQuery 38    │   123.82 ms │   119.64 ms │     no change │
│ QQuery 39    │   199.18 ms │   190.88 ms │     no change │
│ QQuery 40    │    47.56 ms │    44.04 ms │ +1.08x faster │
│ QQuery 41    │    42.18 ms │    42.97 ms │     no change │
│ QQuery 42    │    36.59 ms │    37.29 ms │     no change │
└──────────────┴─────────────┴─────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary          ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)          │ 94950.38ms │
│ Total Time (issue_19286)   │ 94494.42ms │
│ Average Time (HEAD)        │  2208.15ms │
│ Average Time (issue_19286) │  2197.54ms │
│ Queries Faster             │          4 │
│ Queries Slower             │          1 │
│ Queries with No Change     │         38 │
│ Queries with Failure       │          0 │
└────────────────────────────┴────────────┘
--------------------
Benchmark tpch_mem_sf1.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃      HEAD ┃ issue_19286 ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 1     │ 134.20 ms │   136.34 ms │     no change │
│ QQuery 2     │  28.81 ms │    30.21 ms │     no change │
│ QQuery 3     │  40.18 ms │    40.19 ms │     no change │
│ QQuery 4     │  29.77 ms │    30.70 ms │     no change │
│ QQuery 5     │  89.80 ms │    90.01 ms │     no change │
│ QQuery 6     │  20.38 ms │    20.15 ms │     no change │
│ QQuery 7     │ 237.37 ms │   230.91 ms │     no change │
│ QQuery 8     │  36.99 ms │    38.81 ms │     no change │
│ QQuery 9     │ 109.06 ms │   107.79 ms │     no change │
│ QQuery 10    │  65.23 ms │    65.33 ms │     no change │
│ QQuery 11    │  19.01 ms │    18.58 ms │     no change │
│ QQuery 12    │  52.76 ms │    51.70 ms │     no change │
│ QQuery 13    │  48.84 ms │    50.40 ms │     no change │
│ QQuery 14    │  14.48 ms │    15.95 ms │  1.10x slower │
│ QQuery 15    │  24.77 ms │    24.87 ms │     no change │
│ QQuery 16    │  27.06 ms │    26.55 ms │     no change │
│ QQuery 17    │ 164.84 ms │   153.12 ms │ +1.08x faster │
│ QQuery 18    │ 281.56 ms │   285.74 ms │     no change │
│ QQuery 19    │  38.84 ms │    39.09 ms │     no change │
│ QQuery 20    │  50.59 ms │    50.12 ms │     no change │
│ QQuery 21    │ 326.03 ms │   327.57 ms │     no change │
│ QQuery 22    │  18.84 ms │    18.21 ms │     no change │
└──────────────┴───────────┴─────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Benchmark Summary          ┃           ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ Total Time (HEAD)          │ 1859.43ms │
│ Total Time (issue_19286)   │ 1852.34ms │
│ Average Time (HEAD)        │   84.52ms │
│ Average Time (issue_19286) │   84.20ms │
│ Queries Faster             │         1 │
│ Queries Slower             │         1 │
│ Queries with No Change     │        20 │
│ Queries with Failure       │         0 │
└────────────────────────────┴───────────┘

[alamb-ghbot]

🤖 ./gh_compare_branch.sh gh_compare_branch.sh Running Linux aal-dev 6.14.0-1018-gcp #19~24.04.1-Ubuntu SMP Wed Sep 24 23:23:09 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Comparing issue_19286 (d9af260747d28f0188ec7c571c68c3b36d988182) to e914935660875bc415887ade3fea1f5e59bed5e1 diff using: external_aggr Results will be posted here when complete

[alamb-ghbot]

🤖 ./gh_compare_branch.sh gh_compare_branch.sh Running Linux aal-dev 6.14.0-1018-gcp #19~24.04.1-Ubuntu SMP Wed Sep 24 23:23:09 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Comparing issue_19286 (d9af260747d28f0188ec7c571c68c3b36d988182) to e914935660875bc415887ade3fea1f5e59bed5e1 diff using: external_aggr Results will be posted here when complete

[alamb-ghbot]

🤖 ./gh_compare_branch.sh gh_compare_branch.sh Running Linux aal-dev 6.14.0-1018-gcp #19~24.04.1-Ubuntu SMP Wed Sep 24 23:23:09 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Comparing issue_19286 (d9af260747d28f0188ec7c571c68c3b36d988182) to e914935660875bc415887ade3fea1f5e59bed5e1 diff using: external_aggr Results will be posted here when complete

[pepijnve]

Benchmark runner seems to be struggling

[alamb]

Benchmark runner seems to be struggling

It seems that the benchmark doesn't complete on main (I haven't tried on this branch)

     Running `/Users/andrewlamb/Software/datafusion/target/release/external_aggr benchmark --partitions 4 --iterations 5 --path /Users/andrewlamb/Software/datafusion/benchmarks/data/tpch_sf1 -o /Users/andrewlamb/Software/datafusion/benchmarks/results/main/external_aggr.json`
Q1(64.0 MB) iteration 0 took 66.5 ms and returned 1 rows
Q1(64.0 MB) iteration 1 took 40.4 ms and returned 1 rows
Q1(64.0 MB) iteration 2 took 41.3 ms and returned 1 rows
Q1(64.0 MB) iteration 3 took 43.1 ms and returned 1 rows
Q1(64.0 MB) iteration 4 took 40.6 ms and returned 1 rows
Q1(64.0 MB) avg time: 46.38 ms
Error: Shared(ResourcesExhausted("Failed to allocate additional 1850.4 KB for GroupedHashAggregateStream[1] () with 1812.6 KB already allocated for this reservation - 2.7 MB remain available for the total pool"))

[pepijnve]

Interesting. I’ll have a look at that tomorrow.

[pepijnve]

I've taken a less conservative approach in my last commit in an attempt to make the OOM handling strategy easier to follow. There's more code inlined into the poll_next implementation now making that method a bit longer, but I think it's easier to understand this way. @alamb I haven't had a chance to verify if the benchmarks now do pass yet. I'll do that next.

[alamb]

run benchmarks

[alamb-ghbot]

🤖 ./gh_compare_branch.sh gh_compare_branch.sh Running Linux aal-dev 6.14.0-1018-gcp #19~24.04.1-Ubuntu SMP Wed Sep 24 23:23:09 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Comparing issue_19286 (c01a68a5e6c6c449bec545d8d5840a1f8ea290ab) to 2e95f2673ff47c2fc957020d7aca80a27628f472 diff using: tpch_mem clickbench_partitioned clickbench_extended Results will be posted here when complete

[pepijnve]

Regarding the external_aggr benchmark, for me it already fails to run on main so I don't think this is a regression. I'll try to see if we can fix this next.

[alamb-ghbot]

🤖: Benchmark completed

Details

Comparing HEAD and issue_19286
--------------------
Benchmark clickbench_extended.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Query        ┃        HEAD ┃ issue_19286 ┃    Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ QQuery 0     │  2836.89 ms │  2798.95 ms │ no change │
│ QQuery 1     │  1290.73 ms │  1303.25 ms │ no change │
│ QQuery 2     │  2528.97 ms │  2473.68 ms │ no change │
│ QQuery 3     │  1172.05 ms │  1142.70 ms │ no change │
│ QQuery 4     │  2340.80 ms │  2357.78 ms │ no change │
│ QQuery 5     │ 29450.83 ms │ 29413.10 ms │ no change │
│ QQuery 6     │  3893.82 ms │  3978.39 ms │ no change │
│ QQuery 7     │  3844.76 ms │  3737.40 ms │ no change │
└──────────────┴─────────────┴─────────────┴───────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary          ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)          │ 47358.85ms │
│ Total Time (issue_19286)   │ 47205.26ms │
│ Average Time (HEAD)        │  5919.86ms │
│ Average Time (issue_19286) │  5900.66ms │
│ Queries Faster             │          0 │
│ Queries Slower             │          0 │
│ Queries with No Change     │          8 │
│ Queries with Failure       │          0 │
└────────────────────────────┴────────────┘
--------------------
Benchmark clickbench_partitioned.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃        HEAD ┃ issue_19286 ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0     │     2.18 ms │     2.33 ms │  1.07x slower │
│ QQuery 1     │    51.91 ms │    50.73 ms │     no change │
│ QQuery 2     │   143.37 ms │   142.11 ms │     no change │
│ QQuery 3     │   155.14 ms │   159.57 ms │     no change │
│ QQuery 4     │  1131.44 ms │  1155.16 ms │     no change │
│ QQuery 5     │  1508.29 ms │  1555.65 ms │     no change │
│ QQuery 6     │     2.18 ms │     2.25 ms │     no change │
│ QQuery 7     │    57.23 ms │    57.08 ms │     no change │
│ QQuery 8     │  1458.32 ms │  1474.71 ms │     no change │
│ QQuery 9     │  1882.57 ms │  1926.79 ms │     no change │
│ QQuery 10    │   361.95 ms │   365.36 ms │     no change │
│ QQuery 11    │   417.80 ms │   428.94 ms │     no change │
│ QQuery 12    │  1374.91 ms │  1412.49 ms │     no change │
│ QQuery 13    │  2025.58 ms │  2110.27 ms │     no change │
│ QQuery 14    │  1284.20 ms │  1297.21 ms │     no change │
│ QQuery 15    │  1326.58 ms │  1287.19 ms │     no change │
│ QQuery 16    │  2749.32 ms │  2752.64 ms │     no change │
│ QQuery 17    │  2726.51 ms │  2743.32 ms │     no change │
│ QQuery 18    │  5728.27 ms │  5154.52 ms │ +1.11x faster │
│ QQuery 19    │   124.28 ms │   123.58 ms │     no change │
│ QQuery 20    │  1963.88 ms │  1900.65 ms │     no change │
│ QQuery 21    │  2293.42 ms │  2253.69 ms │     no change │
│ QQuery 22    │  3859.63 ms │  3849.41 ms │     no change │
│ QQuery 23    │ 12701.55 ms │ 12415.39 ms │     no change │
│ QQuery 24    │   204.18 ms │   219.63 ms │  1.08x slower │
│ QQuery 25    │   468.83 ms │   463.39 ms │     no change │
│ QQuery 26    │   220.69 ms │   223.55 ms │     no change │
│ QQuery 27    │  2908.46 ms │  2784.59 ms │     no change │
│ QQuery 28    │ 23922.68 ms │ 24701.41 ms │     no change │
│ QQuery 29    │   962.69 ms │   968.42 ms │     no change │
│ QQuery 30    │  1376.78 ms │  1368.46 ms │     no change │
│ QQuery 31    │  1369.13 ms │  1359.51 ms │     no change │
│ QQuery 32    │  6322.97 ms │  5296.08 ms │ +1.19x faster │
│ QQuery 33    │  6749.43 ms │  5997.04 ms │ +1.13x faster │
│ QQuery 34    │  6341.35 ms │  6785.67 ms │  1.07x slower │
│ QQuery 35    │  2067.96 ms │  2001.26 ms │     no change │
│ QQuery 36    │    68.73 ms │    66.96 ms │     no change │
│ QQuery 37    │    46.63 ms │    46.55 ms │     no change │
│ QQuery 38    │    65.47 ms │    68.93 ms │  1.05x slower │
│ QQuery 39    │   101.38 ms │   101.72 ms │     no change │
│ QQuery 40    │    28.53 ms │    26.45 ms │ +1.08x faster │
│ QQuery 41    │    23.54 ms │    23.84 ms │     no change │
│ QQuery 42    │    20.41 ms │    20.58 ms │     no change │
└──────────────┴─────────────┴─────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary          ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)          │ 98600.33ms │
│ Total Time (issue_19286)   │ 97145.10ms │
│ Average Time (HEAD)        │  2293.03ms │
│ Average Time (issue_19286) │  2259.19ms │
│ Queries Faster             │          4 │
│ Queries Slower             │          4 │
│ Queries with No Change     │         35 │
│ Queries with Failure       │          0 │
└────────────────────────────┴────────────┘
--------------------
Benchmark tpch_mem_sf1.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃      HEAD ┃ issue_19286 ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 1     │ 144.12 ms │   141.09 ms │     no change │
│ QQuery 2     │  29.65 ms │    30.92 ms │     no change │
│ QQuery 3     │  35.15 ms │    40.83 ms │  1.16x slower │
│ QQuery 4     │  29.35 ms │    30.08 ms │     no change │
│ QQuery 5     │  90.96 ms │    89.65 ms │     no change │
│ QQuery 6     │  20.10 ms │    20.11 ms │     no change │
│ QQuery 7     │ 237.76 ms │   259.68 ms │  1.09x slower │
│ QQuery 8     │  36.82 ms │    38.36 ms │     no change │
│ QQuery 9     │ 111.21 ms │   119.90 ms │  1.08x slower │
│ QQuery 10    │  65.79 ms │    65.90 ms │     no change │
│ QQuery 11    │  17.88 ms │    18.52 ms │     no change │
│ QQuery 12    │  52.24 ms │    53.32 ms │     no change │
│ QQuery 13    │  47.78 ms │    49.95 ms │     no change │
│ QQuery 14    │  14.38 ms │    14.86 ms │     no change │
│ QQuery 15    │  24.74 ms │    25.28 ms │     no change │
│ QQuery 16    │  26.17 ms │    27.60 ms │  1.05x slower │
│ QQuery 17    │ 155.89 ms │   163.55 ms │     no change │
│ QQuery 18    │ 301.87 ms │   297.74 ms │     no change │
│ QQuery 19    │  47.81 ms │    37.74 ms │ +1.27x faster │
│ QQuery 20    │  64.98 ms │    52.47 ms │ +1.24x faster │
│ QQuery 21    │ 332.02 ms │   331.67 ms │     no change │
│ QQuery 22    │  18.32 ms │    18.26 ms │     no change │
└──────────────┴───────────┴─────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Benchmark Summary          ┃           ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ Total Time (HEAD)          │ 1904.99ms │
│ Total Time (issue_19286)   │ 1927.50ms │
│ Average Time (HEAD)        │   86.59ms │
│ Average Time (issue_19286) │   87.61ms │
│ Queries Faster             │         2 │
│ Queries Slower             │         4 │
│ Queries with No Change     │        16 │
│ Queries with Failure       │         0 │
└────────────────────────────┴───────────┘

[alamb]

Regarding the external_aggr benchmark, for me it already fails to run on main so I don't think this is a regression.

Agreed

I'll try to see if we can fix this next.

❤️

[alamb]

run benchmarks

[alamb]

I don't think the benchmarks show any actual peformance change, but I am rerunning them to be sure

[alamb-ghbot]

🤖 ./gh_compare_branch.sh gh_compare_branch.sh Running Linux aal-dev 6.14.0-1018-gcp #19~24.04.1-Ubuntu SMP Wed Sep 24 23:23:09 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Comparing issue_19286 (c01a68a5e6c6c449bec545d8d5840a1f8ea290ab) to 2e95f2673ff47c2fc957020d7aca80a27628f472 diff using: tpch_mem clickbench_partitioned clickbench_extended Results will be posted here when complete

[alamb-ghbot]

🤖: Benchmark completed

Details

Comparing HEAD and issue_19286
--------------------
Benchmark clickbench_extended.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Query        ┃        HEAD ┃ issue_19286 ┃    Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ QQuery 0     │  2768.82 ms │  2765.70 ms │ no change │
│ QQuery 1     │  1313.97 ms │  1314.59 ms │ no change │
│ QQuery 2     │  2429.85 ms │  2455.74 ms │ no change │
│ QQuery 3     │  1151.88 ms │  1149.40 ms │ no change │
│ QQuery 4     │  2323.83 ms │  2286.93 ms │ no change │
│ QQuery 5     │ 28922.46 ms │ 29214.73 ms │ no change │
│ QQuery 6     │  3895.29 ms │  3930.29 ms │ no change │
│ QQuery 7     │  3785.83 ms │  3882.16 ms │ no change │
└──────────────┴─────────────┴─────────────┴───────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary          ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)          │ 46591.94ms │
│ Total Time (issue_19286)   │ 46999.55ms │
│ Average Time (HEAD)        │  5823.99ms │
│ Average Time (issue_19286) │  5874.94ms │
│ Queries Faster             │          0 │
│ Queries Slower             │          0 │
│ Queries with No Change     │          8 │
│ Queries with Failure       │          0 │
└────────────────────────────┴────────────┘
--------------------
Benchmark clickbench_partitioned.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃        HEAD ┃ issue_19286 ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0     │     2.22 ms │     2.74 ms │  1.23x slower │
│ QQuery 1     │    50.23 ms │    51.30 ms │     no change │
│ QQuery 2     │   133.81 ms │   139.80 ms │     no change │
│ QQuery 3     │   157.18 ms │   159.65 ms │     no change │
│ QQuery 4     │  1092.15 ms │  1153.74 ms │  1.06x slower │
│ QQuery 5     │  1543.84 ms │  1560.65 ms │     no change │
│ QQuery 6     │     2.14 ms │     2.38 ms │  1.11x slower │
│ QQuery 7     │    54.53 ms │    57.03 ms │     no change │
│ QQuery 8     │  1479.46 ms │  1502.51 ms │     no change │
│ QQuery 9     │  1958.82 ms │  1917.22 ms │     no change │
│ QQuery 10    │   410.71 ms │   364.92 ms │ +1.13x faster │
│ QQuery 11    │   442.14 ms │   423.12 ms │     no change │
│ QQuery 12    │  1397.93 ms │  1412.01 ms │     no change │
│ QQuery 13    │  2060.94 ms │  2073.17 ms │     no change │
│ QQuery 14    │  1286.09 ms │  1280.82 ms │     no change │
│ QQuery 15    │  1277.17 ms │  1312.64 ms │     no change │
│ QQuery 16    │  2757.65 ms │  2767.15 ms │     no change │
│ QQuery 17    │  2729.83 ms │  2717.40 ms │     no change │
│ QQuery 18    │  5939.06 ms │  5162.20 ms │ +1.15x faster │
│ QQuery 19    │   125.56 ms │   124.99 ms │     no change │
│ QQuery 20    │  1917.38 ms │  1907.23 ms │     no change │
│ QQuery 21    │  2251.86 ms │  2208.20 ms │     no change │
│ QQuery 22    │  3889.01 ms │  3832.07 ms │     no change │
│ QQuery 23    │ 13275.21 ms │ 12340.19 ms │ +1.08x faster │
│ QQuery 24    │   224.05 ms │   212.70 ms │ +1.05x faster │
│ QQuery 25    │   461.66 ms │   476.05 ms │     no change │
│ QQuery 26    │   213.19 ms │   223.85 ms │  1.05x slower │
│ QQuery 27    │  2866.67 ms │  2782.98 ms │     no change │
│ QQuery 28    │ 23819.06 ms │ 24724.33 ms │     no change │
│ QQuery 29    │   958.48 ms │   960.03 ms │     no change │
│ QQuery 30    │  1366.33 ms │  1330.67 ms │     no change │
│ QQuery 31    │  1375.98 ms │  1341.69 ms │     no change │
│ QQuery 32    │  5207.21 ms │  5163.16 ms │     no change │
│ QQuery 33    │  6203.72 ms │  6054.05 ms │     no change │
│ QQuery 34    │  6418.46 ms │  5980.95 ms │ +1.07x faster │
│ QQuery 35    │  1981.89 ms │  1949.05 ms │     no change │
│ QQuery 36    │    68.58 ms │    68.12 ms │     no change │
│ QQuery 37    │    45.66 ms │    46.57 ms │     no change │
│ QQuery 38    │    66.52 ms │    69.46 ms │     no change │
│ QQuery 39    │   105.19 ms │   105.74 ms │     no change │
│ QQuery 40    │    26.53 ms │    28.57 ms │  1.08x slower │
│ QQuery 41    │    24.16 ms │    24.53 ms │     no change │
│ QQuery 42    │    20.46 ms │    20.55 ms │     no change │
└──────────────┴─────────────┴─────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary          ┃            ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (HEAD)          │ 97688.73ms │
│ Total Time (issue_19286)   │ 96036.21ms │
│ Average Time (HEAD)        │  2271.83ms │
│ Average Time (issue_19286) │  2233.40ms │
│ Queries Faster             │          5 │
│ Queries Slower             │          5 │
│ Queries with No Change     │         33 │
│ Queries with Failure       │          0 │
└────────────────────────────┴────────────┘
--------------------
Benchmark tpch_mem_sf1.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━┓
┃ Query        ┃      HEAD ┃ issue_19286 ┃       Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━┩
│ QQuery 1     │ 138.48 ms │   141.64 ms │    no change │
│ QQuery 2     │  28.19 ms │    28.36 ms │    no change │
│ QQuery 3     │  36.25 ms │    38.92 ms │ 1.07x slower │
│ QQuery 4     │  28.96 ms │    29.67 ms │    no change │
│ QQuery 5     │  90.55 ms │    91.09 ms │    no change │
│ QQuery 6     │  19.93 ms │    20.31 ms │    no change │
│ QQuery 7     │ 228.84 ms │   240.87 ms │ 1.05x slower │
│ QQuery 8     │  35.72 ms │    38.25 ms │ 1.07x slower │
│ QQuery 9     │ 108.47 ms │   111.29 ms │    no change │
│ QQuery 10    │  66.39 ms │    63.70 ms │    no change │
│ QQuery 11    │  18.86 ms │    18.68 ms │    no change │
│ QQuery 12    │  52.14 ms │    52.21 ms │    no change │
│ QQuery 13    │  46.80 ms │    49.45 ms │ 1.06x slower │
│ QQuery 14    │  14.08 ms │    13.93 ms │    no change │
│ QQuery 15    │  25.02 ms │    25.20 ms │    no change │
│ QQuery 16    │  25.06 ms │    25.40 ms │    no change │
│ QQuery 17    │ 154.48 ms │   156.04 ms │    no change │
│ QQuery 18    │ 287.30 ms │   287.54 ms │    no change │
│ QQuery 19    │  38.08 ms │    37.04 ms │    no change │
│ QQuery 20    │  50.96 ms │    50.11 ms │    no change │
│ QQuery 21    │ 309.00 ms │   311.44 ms │    no change │
│ QQuery 22    │  18.31 ms │    18.11 ms │    no change │
└──────────────┴───────────┴─────────────┴──────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━┓
┃ Benchmark Summary          ┃           ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━┩
│ Total Time (HEAD)          │ 1821.86ms │
│ Total Time (issue_19286)   │ 1849.24ms │
│ Average Time (HEAD)        │   82.81ms │
│ Average Time (issue_19286) │   84.06ms │
│ Queries Faster             │         0 │
│ Queries Slower             │         4 │
│ Queries with No Change     │        18 │
│ Queries with Failure       │         0 │
└────────────────────────────┴───────────┘

[pepijnve]

@alamb https://github.com/apache/datafusion/pull/19287/commits/9f2f22ef0c7ecfd2d8a504308079c1831be79949 makes the external_aggr benchmark succeed.

Because I was setting can spill: false for OutOfMemoryMode::EmitEarly, there was in effect still greedy memory reservation despite the use of FairSpillPool. That was causing one of the streams to not be able to reserve even a minuscule amount of memory.

Additionally the 'emit early' code would only kick in if there was at least one full batch of group values to emit. I've modified the code to still try to do that if it can, but fall back to emitting a smaller batch if memory pressure requires it.

[pepijnve]

One remaining question I have is how skip_aggregation_probe and group_values_soft_limit are expected to interact with the memory reservation system.

In SkipAggregationProbe::update_state the total number of input rows is accumulated, but the only the current number of group values is compared against that. If disk spilling or early emission is in effect, then the consequence is that the ratio will only decrease since the denominator of the ratio keeps on increasing. In other words, it's unlikely to ever kick in.

The same goes for group_values_soft_limit. It's compared against the number of group values that are held in memory by the aggregation stream itself. If we keep flushing values either down the pipeline or to disk, it's also unlikely to ever trigger.

[alamb]

I think this one is ready to go. I'll just update it once more and run benchmarks one more time to make me feel better and plan to merge it in

[alamb]

run benchmark clickbench_partitioned

[alamb-ghbot]

🤖 ./gh_compare_branch.sh gh_compare_branch.sh Running Linux aal-dev 6.14.0-1018-gcp #19~24.04.1-Ubuntu SMP Wed Sep 24 23:23:09 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Comparing issue_19286 (2ea0e3debd644ed27fffbb6ab7cab922d45693b7) to ead8209803770773980fafaf0fc622bb606be0ee diff using: clickbench_partitioned Results will be posted here when complete

[alamb]

run benchmark tpch

[alamb-ghbot]

🤖: Benchmark completed

Details

Comparing HEAD and issue_19286
--------------------
Benchmark clickbench_partitioned.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query        ┃        HEAD ┃ issue_19286 ┃        Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0     │     2.29 ms │     2.54 ms │  1.11x slower │
│ QQuery 1     │    49.15 ms │    49.71 ms │     no change │
│ QQuery 2     │   135.30 ms │   131.44 ms │     no change │
│ QQuery 3     │   158.94 ms │   152.10 ms │     no change │
│ QQuery 4     │  1077.08 ms │  1094.73 ms │     no change │
│ QQuery 5     │  1504.96 ms │  1499.92 ms │     no change │
│ QQuery 6     │     2.17 ms │     2.10 ms │     no change │
│ QQuery 7     │    55.89 ms │    55.15 ms │     no change │
│ QQuery 8     │  1444.52 ms │  1446.30 ms │     no change │
│ QQuery 9     │  1867.87 ms │  1866.52 ms │     no change │
│ QQuery 10    │   362.73 ms │   362.69 ms │     no change │
│ QQuery 11    │   415.56 ms │   423.89 ms │     no change │
│ QQuery 12    │  1348.89 ms │  1347.85 ms │     no change │
│ QQuery 13    │  2007.19 ms │  2053.80 ms │     no change │
│ QQuery 14    │  1286.53 ms │  1277.48 ms │     no change │
│ QQuery 15    │  1266.86 ms │  1283.56 ms │     no change │
│ QQuery 16    │  2680.37 ms │  2685.34 ms │     no change │
│ QQuery 17    │  2667.04 ms │  2684.80 ms │     no change │
│ QQuery 18    │  5779.79 ms │  4956.34 ms │ +1.17x faster │
│ QQuery 19    │   123.39 ms │   122.35 ms │     no change │
│ QQuery 20    │  1979.79 ms │  1893.95 ms │     no change │
│ QQuery 21    │  2249.46 ms │  2178.91 ms │     no change │
│ QQuery 22    │  3832.69 ms │  3714.92 ms │     no change │
│ QQuery 23    │ 18395.94 ms │ 12136.24 ms │ +1.52x faster │
│ QQuery 24    │   215.60 ms │   212.90 ms │     no change │
│ QQuery 25    │   463.47 ms │   452.82 ms │     no change │
│ QQuery 26    │   227.38 ms │   226.94 ms │     no change │
│ QQuery 27    │  2828.96 ms │  2760.40 ms │     no change │
│ QQuery 28    │ 25164.08 ms │ 21938.93 ms │ +1.15x faster │
│ QQuery 29    │   964.94 ms │   974.72 ms │     no change │
│ QQuery 30    │  1364.88 ms │  1316.22 ms │     no change │
│ QQuery 31    │  1356.24 ms │  1322.71 ms │     no change │
│ QQuery 32    │  5098.17 ms │  5120.02 ms │     no change │
│ QQuery 33    │  6036.11 ms │  5869.18 ms │     no change │
│ QQuery 34    │  6409.47 ms │  6356.96 ms │     no change │
│ QQuery 35    │  1986.02 ms │  1939.25 ms │     no change │
│ QQuery 36    │    66.26 ms │    70.39 ms │  1.06x slower │
│ QQuery 37    │    45.43 ms │    44.82 ms │     no change │
│ QQuery 38    │    67.59 ms │    66.48 ms │     no change │
│ QQuery 39    │   105.42 ms │   100.53 ms │     no change │
│ QQuery 40    │    27.78 ms │    26.24 ms │ +1.06x faster │
│ QQuery 41    │    24.04 ms │    23.77 ms │     no change │
│ QQuery 42    │    21.02 ms │    19.77 ms │ +1.06x faster │
└──────────────┴─────────────┴─────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━┓
┃ Benchmark Summary          ┃             ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━━┩
│ Total Time (HEAD)          │ 103167.27ms │
│ Total Time (issue_19286)   │  92265.70ms │
│ Average Time (HEAD)        │   2399.24ms │
│ Average Time (issue_19286) │   2145.71ms │
│ Queries Faster             │           5 │
│ Queries Slower             │           2 │
│ Queries with No Change     │          36 │
│ Queries with Failure       │           0 │
└────────────────────────────┴─────────────┘