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]

Reply via email to