xiangfu0 commented on code in PR #20228:
URL: https://github.com/apache/pinot/pull/20228#discussion_r4541615392
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/customobject/CustomObjectAccumulator.java:
##########
@@ -22,34 +22,27 @@
import java.util.ArrayList;
-/// Intermediate state used by some aggregation functions which gives the end
user more control over how custom objects
-/// are merged for performance reasons. Some custom objects such as
DataSketches have better merge performance when
-/// more than two items are merged at once through the elimination of
intermediate bookkeeping overheads.
-///
-/// The end user can set a value for the "threshold" parameter that defers the
merge operation until at least as many
-/// items are ready to be merged, or the callee forces the merge directly via
"getResult" - e.g. on serialization.
-/// This data structure trades-off more memory usage for a greater degree of
pre-aggregation in the accumulator state.
+/// Buffers intermediate values until the configured threshold is reached or a
result is requested.
+/// Subclasses merge pending values in [#flush()]. Instances are mutable and
not thread-safe.
public abstract class CustomObjectAccumulator<T> {
protected ArrayList<T> _accumulator;
- private int _threshold;
+ private final int _threshold;
private int _numInputs = 0;
- public CustomObjectAccumulator() {
- this(2);
- }
-
- public CustomObjectAccumulator(int threshold) {
- setThreshold(threshold);
- }
-
- /// Sets the threshold that determines how much memory to use for the
internal accumulator state before
- /// the intermediate state is merged.
- /// @param threshold the threshold \[> 0\].
- public void setThreshold(int threshold) {
+ protected CustomObjectAccumulator(int threshold) {
Review Comment:
FYI, non-blocking: this commit removes public API from pinot-segment-local.
That includes the no-arg accumulator constructors, `setThreshold`,
`setLgNominalEntries`, `setSetOperationBuilder`, `setNominalEntries`, and
`setSetOperations`. This constructor is also now protected. The binary-compat
job only covers pinot-spi and pinot-segment-spi, so it won't flag this. In-tree
callers are all updated; consider a release note for out-of-tree custom
aggregation functions.
--
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]
