dlscjf151 commented on code in PR #20171:
URL: https://github.com/apache/pinot/pull/20171#discussion_r4490239992
##########
pinot-core/src/main/java/org/apache/pinot/core/routing/timeboundary/TimeBoundaryStrategyService.java:
##########
@@ -57,11 +57,12 @@ public static void setInstance(TimeBoundaryStrategyService
service) {
_instance = service;
}
+ /// Returns a new strategy instance so logical-table-specific initialization
is not shared between requests.
Review Comment:
Thanks, addressed in b4cbc3a0bb2cb9891992c56c6c75e85dd88a5c95.
The `TimeBoundaryStrategy` interface now documents that instances are
created per lookup, public no-argument constructors should be inexpensive,
logical-table-specific state belongs in `init()`, and any shared caches or
resources must be thread-safe. This follow-up changes documentation only; the
implementation is unchanged.
I also corrected the PR and linked issue to cover hybrid logical-table
routing in both SSE and MSE.
Validation on JDK 25:
```sh
mvn -B -ntp -pl pinot-core -am \
-Dtest=MinTimeBoundaryStrategyTest,TimeBoundaryStrategyServiceTest \
-Dsurefire.failIfNoSpecifiedTests=false test checkstyle:check
```
All 16 selected reactor modules succeeded; 7 tests passed with no failures,
errors, or skips. Javadoc generation also succeeded, and the rendered interface
documentation contains the lifecycle, constructor, initialization, and
shared-resource guidance.
Tooling disclosure: this reply was prepared with OpenAI coding-assistant
assistance.
--
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]
