mrhhsg commented on code in PR #68564:
URL: https://github.com/apache/doris/pull/68564#discussion_r4121069375
##########
be/src/exec/pipeline/dependency.h:
##########
@@ -461,6 +461,12 @@ struct BucketedAggSharedState : public BasicSharedState {
/// Element i is true when instance i's data for this bucket has been
merged.
/// Sized to num_sink_instances in init_instances().
std::vector<bool> merged_instances;
+ /// Arena for memory allocated by aggregate function merges into this
bucket.
+ /// Accessed only under merge_in_progress CAS lock. Sink instance
arenas cannot be
+ /// used here because different buckets are merged concurrently by
different
+ /// source instances and Arena is not thread-safe. Merged states may
point into
+ /// this arena, so it must live as long as the shared state.
+ Arena arena;
Review Comment:
Fixed in 3f19ae3c04c. The per-bucket arenas are gone. Each source instance
now has its own merge arena, stored in
`BucketedAggSharedState::source_merge_arenas`:
- **Where they are created:** `init_instances()`, one for each source
dependency.
- **How a source finds its arena:** by its source task index.
- **Why a source task can use it without a lock:** a pipeline task runs on
only one thread at a time, so it has exclusive use of its arena even though
different buckets are merged concurrently.
- **Lifetime:** the arenas live as long as the shared state, because merged
states may point into them and any source instance may output a bucket.
- **Memory floor:** at most one lazily allocated arena per source instance
that actually merges something. The floor now scales with source parallelism,
not with the 256 buckets.
I also added a BE UT, `BucketedAggSharedStateTest`, and re-ran the bucketed
agg regression suites (`collect_set_bucketed_agg_merge` 3 times,
`percentile_bucketed_agg_merge`, `bucketed_hash_agg`). They pass.
--
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]