dlscjf151 opened a new pull request, #20171:
URL: https://github.com/apache/pinot/pull/20171
## Summary
- Cache JDK `ServiceLoader.Provider<TimeBoundaryStrategy>` objects, not
mutable strategy instances.
- Create a fresh strategy on every lookup so each logical-table request owns
its initialized state.
- Add a deterministic regression covering two logical tables initialized
before either boundary is computed.
## Problem
`LogicalTableRouteProvider` retrieves a strategy, calls
`init(logicalTableConfig, tableCache)`, and computes the boundary later. The
registry currently returns a shared instance for each strategy name.
For `MinTimeBoundaryStrategy`, another request can replace or mutate
`_dateTimeFormatSpecMap` between initialization and computation. This can
return another logical table's boundary or no boundary. A null boundary causes
logical routing to omit OFFLINE tables without necessarily reporting a query
error.
The bug affects single-stage hybrid logical tables, including federation,
rather than only multi-cluster deployments.
## Implementation
The service registry remains a singleton. Only strategy instances become
request-scoped. The JDK service-provider mechanism handles creation; no custom
reflection/constructor handling, new dependency, global serialization, or
strategy-interface change is introduced.
The existing strategy-name discovery, duplicate-name rejection, and
unknown-name error remain unchanged. The strategy lookup method documents its
instance lifetime.
## Verification
### Current upstream Maven build
Executed on JDK 25 against the current 1.7.0-SNAPSHOT reactor:
```sh
mvn -B -ntp -pl pinot-core -am \
-Dtest=MinTimeBoundaryStrategyTest,TimeBoundaryStrategyServiceTest \
-Dsurefire.failIfNoSpecifiedTests=false test
```
Result: `BUILD SUCCESS`; all 16 selected reactor modules succeeded. Tests
run: 7, failures: 0, errors: 0, skipped: 0. This is a targeted reactor
build/test run, not the complete repository test suite.
### Focused regression
Compiled the upstream strategy interface, service,
`MinTimeBoundaryStrategy`, two test classes, and `LogicalTableRouteProvider`
directly with `javac --release 17 -proc:none`. Executed the tests on Java 21
with an existing local Pinot 1.5 test dependency classpath.
TestNG entry point: `org.testng.TestNG`, selecting
`MinTimeBoundaryStrategyTest` and `TimeBoundaryStrategyServiceTest`.
- Before: 7 tests, 6 passed, 1 failed. The new
`testConfiguredViewsDoNotShareBoundaryTables` failed because request A computed
request B's boundary.
- After: 7 tests, 7 passed, 0 failed or skipped.
This direct compilation provided the before/after reproduction;
current-upstream Maven verification is recorded above.
### Manual concurrent routing smoke
Used the real upstream `LogicalTableRouteProvider`, real strategy
implementations, real logical-table configurations and schemas/date-time format
conversion, with stable mocked external `TableCache` and `RoutingManager`
inputs.
- Before: the deterministic metadata(A), metadata(B), route(A), route(B)
interleaving returned the wrong boundary.
- After: that interleaving passed. An additional 10,000
`getTableRouteInfo()` calls on 24 worker threads across two logical tables all
retained their expected time boundary and OFFLINE route.
- The same 10,000-call smoke also passed on JDK 25 using the Maven-built
current upstream 1.7.0-SNAPSHOT classes and dependency classpath.
This was a Java routing-API smoke, not a running broker or SQL/network
integration test.
## Scope
Source baseline: Apache Pinot master
`0a53497fddfc61220feabc5b8f62516f4183525f`.
Only the strategy service and the existing strategy test class are changed.
No deployment configuration, company-specific tables, infrastructure addresses,
or operational credentials are included.
## Related issue
Fixes https://github.com/apache/pinot/issues/20170
## Tooling disclosure
OpenAI coding-assistant assistance was used to prepare the implementation,
regression test, verification runs, and this PR text. The observed verification
scope and limitations are recorded above.
--
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]
