mrhhsg opened a new pull request, #67140:
URL: https://github.com/apache/doris/pull/67140
### What problem does this PR solve?
Issue Number: None
Problem Summary:
Streaming pre-aggregation (`StreamingAggOperatorX`) only limited its hash
table
memory when spilling was enabled, using the fixed session variable
`spill_streaming_agg_mem_limit` per task; without spilling there was no
memory
gate at all, so a first-phase agg with high cardinality could grow its hash
tables until the query hit its memory limit. `DistinctStreamingAggOperatorX`
had no memory gate at all, only the `low_memory_mode()` switch.
This PR derives the per-task pre-aggregation budget from the live query
memory
limit instead (`streaming_agg_memory_limit()` in
`exec/operator/streaming_agg_memory_limit.h`):
- budget = query mem_limit / parallel_tasks / 5, re-read on every block so a
limit lowered or restored by the workload group manager takes effect at
once;
- the budget never drops below twice the last cache tier of the min-reduction
table (32MB) so a small query limit does not disable pre-aggregation
altogether, but the floor is capped by the per-task share of the query
limit;
- `spill_streaming_agg_mem_limit` becomes an explicit upper bound applied on
top, whether or not spilling is enabled (0 = no explicit bound).
Both operators use the same helper, so the distinct pre-agg now honours the
same rule. The low-memory switch of `StreamingAggOperatorX` is turned into an
`std::atomic_bool` (it was a plain `size_t` written by any pipeline task
while
others read it), `_spill_streaming_agg_mem_limit` gets an initializer (the
mock
operator in unit tests never calls `init()`), and the effective limit is
exposed in the profile as `MemoryUseLimit`.
### Release note
Streaming pre-aggregation memory is now bounded by one fifth of the query
memory limit per operator (shared among its parallel tasks, floor 32MB per
task); `spill_streaming_agg_mem_limit` applies as an explicit upper bound
regardless of `enable_spill`.
### Check List (For Author)
- Test
- [x] Unit Test
- [ ] Regression test
- [ ] Manual test
- [ ] No need to test or manual test
- Behavior changed:
- [x] Yes.
- non-spill queries: the pre-agg hash table is now capped at
`mem_limit / parallel_tasks / 5` (min 32MB, max
`spill_streaming_agg_mem_limit`) per task instead of unbounded.
- spill-enabled queries: the cap is
`min(spill_streaming_agg_mem_limit,
dynamic budget)` instead of the fixed session value.
- Does this need documentation?
- [x] Yes. `spill_streaming_agg_mem_limit` now applies regardless of
`enable_spill`.
### Validation
```
./run-be-ut.sh -j 160 --run
--filter='StreamingAggMemoryLimitTest.*:StreamingAggOperatorTest.*:DistinctStreamingAggOperatorTest.*'
[ PASSED ] 11 tests.
```
- `StreamingAggMemoryLimitTest.budget_floor_and_fixed_limit` (new) covers
the 1/5 rule, the
32MB floor, the floor capped by the per-task share, the explicit bound,
and unknown limits.
- `StreamingAggOperatorTest.test1` /
`DistinctStreamingAggOperatorTest.refresh_memory_limit`
check the effective limit reported in `MemoryUseLimit` while the query
limit changes, the
explicit bound, and that the distinct pre-agg passes rows through once the
limit is exceeded.
- No regression test: the behaviour depends on the query memory limit and
the hash-table size
and cannot be asserted deterministically from SQL output.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
https://claude.ai/code/session_01DzWxKNMrrsmpogSDPEP6Hi
--
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]