eye-gu commented on code in PR #1001:
URL:
https://github.com/apache/skywalking-banyandb/pull/1001#discussion_r3036910617
##########
banyand/dquery/topn.go:
##########
@@ -103,6 +131,10 @@ func (t *topNQueryProcessor) Rev(ctx context.Context,
message bus.Message) (resp
span.Stop()
}()
}
+ originalTopN := request.GetTopN()
+ // Set topN to 0 to disable truncation on data nodes for all
aggregation functions.
+ // This ensures coordinator-side aggregation can see all relevant
candidates.
+ request.TopN = 0
Review Comment:
min/max can be truncated via TopN at the datanode, but sum/count/mean must
read the full dataset to calculate accurate results. To avoid OOM, one approach
is to follow a MapReduce pattern, but even then, sum/count/mean still need to
collect all the pre-calculated data. If you agree, I can open a separate issue
to fix this bug first, and then come back to this PR.
##########
banyand/dquery/topn.go:
##########
@@ -103,6 +131,10 @@ func (t *topNQueryProcessor) Rev(ctx context.Context,
message bus.Message) (resp
span.Stop()
}()
}
+ originalTopN := request.GetTopN()
+ // Set topN to 0 to disable truncation on data nodes for all
aggregation functions.
+ // This ensures coordinator-side aggregation can see all relevant
candidates.
+ request.TopN = 0
Review Comment:
> For high-cardinality entities, this could OOM data nodes during
distributed TopN queries.
min/max can be truncated via TopN at the datanode, but sum/count/mean must
read the full dataset to calculate accurate results. To avoid OOM, one approach
is to follow a MapReduce pattern, but even then, sum/count/mean still need to
collect all the pre-calculated data. If you agree, I can open a separate issue
to fix this bug first, and then come back to this PR.
--
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]
