xiangfu0 commented on code in PR #19539: URL: https://github.com/apache/pinot/pull/19539#discussion_r4190587624
########## pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunction.java: ########## @@ -30,6 +30,8 @@ /// Implementations of this interface are assumed not to be stateful. That is, two invocations of /// `PartitionFunction.getPartition(value)` with the same value are expected to produce the same /// result. Implementations must also be safe for concurrent invocation by multiple threads. +/// Functions with equal identity may reuse partition ids during broker pruning. Implementations that override +/// `equals` and `hashCode` must include every setting that affects partition ids, including subclass state. Review Comment: Intentional. `BoundedColumnValuePartitionFunction` can have a very large configured value array; comparing/hashing it across segments can cost more than direct partition evaluation. I added a default `supportsPartitionIdPreparation()` hook and made Bounded opt out, so the broker uses the original per-segment evaluator for it (096db71a46). The large-config threshold cases pass in the 29-test focused pruner suite. ########## pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunction.java: ########## @@ -30,6 +30,8 @@ /// Implementations of this interface are assumed not to be stateful. That is, two invocations of /// `PartitionFunction.getPartition(value)` with the same value are expected to produce the same /// result. Implementations must also be safe for concurrent invocation by multiple threads. +/// Functions with equal identity may reuse partition ids during broker pruning. Implementations that override Review Comment: Added interface Javadoc in 096db71a46: equality/hash must cover every partition-ID-affecting setting, but the raw config map need not be compared when the parsed fields already capture those settings. Unrelated config entries should not block reuse. -- 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]
