mrhhsg opened a new pull request, #68564:
URL: https://github.com/apache/doris/pull/68564
### What problem does this PR solve?
Issue Number: None
Related PR: #61495
Problem Summary: The source side of bucketed hash aggregation merges the
per-sink-instance hash tables bucket by bucket. The per-bucket CAS lock
(`merge_in_progress`) only serializes work on the same bucket, so two source
tasks can merge two different buckets at the same time. Both of them passed
`*src_inst.arena` (the arena of the sink instance being merged) to
`merge_agg_states` / `merge_null_key`, i.e. the same non-thread-safe `Arena`
was used concurrently by several threads. Aggregate functions whose merge
allocates from the arena (for example `collect_set` on strings through
`arena.insert`, or DISTINCT on strings) then got overlapping memory, which
corrupts the merged states: wrong results and potentially BE crashes.
Reproduce on a single BE with `enable_bucketed_hash_agg=true`,
`parallel_pipeline_task_num=8` and a table whose group keys are present in
all
tablets:
SELECT k, collect_set(s) FROM t GROUP BY k;
Before the fix the total number of collected elements was randomly lower than
expected (e.g. 299431 / 299484 instead of 300000) and differed between runs.
After the fix the result is stable and equal to the non-bucketed plan.
Fix: give every bucket its own `Arena` in `BucketMergeState`. It is only used
by the source task holding that bucket's CAS lock, and it lives as long as
the
shared state because merged states may point into it. The final null-key
merge
keeps using the merge target's arena since it runs single-threaded after all
buckets are done.
### Release note
Fix wrong results or crashes of bucketed hash aggregation with aggregate
functions whose merge allocates memory, such as collect_set on strings.
### Check List (For Author)
- Test:
- Regression test: added
query_p0/aggregate/collect_set_bucketed_agg_merge
(reproduced the wrong result without the fix, passes with it); also ran
percentile_bucketed_agg_merge and agg_strategy/bucketed_hash_agg
- Unit Test: None (the race needs concurrent pipeline tasks; the bucketed
agg operators have no BE UT harness yet)
- Behavior changed: No
- Does this need documentation: No
--
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]
