xiangfu0 opened a new pull request, #18744: URL: https://github.com/apache/pinot/pull/18744
## Problem MSE queries on partitioned tables can fail with: ``` Failed to find enabled fully replicated server for table: X, partition: N ``` `SegmentPartitionMetadataManager` computes the per-partition fully replicated servers by intersecting the ExternalView ONLINE/CONSUMING server sets across all non-"new" segments. Two issues make this fragile: 1. **The "new segment" protection is purely time based** (`pinot.broker.new.segment.expiration.seconds`, made configurable in #18707). Segments that take longer than the window to become fully replicated (e.g. large upsert segments still downloading/preloading), and segments uploaded under a new name to replace other segments (e.g. via merge/rollup or compaction style tasks using the segment replacement protocol), start reducing the fully replicated servers the moment the window lapses — potentially down to the empty set, which fails **every** MSE query on the partition. Raising the config only moves the cliff. 2. **A segment with no available replica clears the partition's fully replicated servers** entirely. No server can serve such a segment regardless of which server is picked, so clearing the set turns one bad segment (all replicas in ERROR state, or a transient deletion race between EV/IS updates) into a full partition outage instead of a partial-data response. ## Fix 1. **Track per-segment replication state against the latest IdealState assignment** (the IdealState was already passed in but unused). A segment that has never been observed fully replicated per its current target assignment is "pending" and excluded from the partition info if it would reduce the fully replicated servers. The pending clock is keyed to when the segment's ideal assignment last changed (covering new segments, replaced segments, and rebalance moves alike), and is still bounded by `pinot.broker.new.segment.expiration.seconds` so a replica permanently failing to load cannot silently exclude a segment from being served forever. Once fully replicated, losing a replica reduces the fully replicated servers as before, so queries are routed to the remaining replicas. When target servers cannot be determined from the ideal state, the previous creation-time based check is used as fallback. 2. **Exclude unavailable segments instead of clearing the partition's fully replicated servers**, and report them in the query response as a `SERVER_SEGMENT_MISSING` exception via `DispatchablePlanMetadata#addUnavailableSegments` — the same mechanism the non-partitioned leaf stage uses, honoring the `ignoreMissingSegments` query option. 3. **Add the missing `pinot-java-client` test dependency to `pinot-broker`.** Since #17167 made `ControllerTest` reference `PinotAdminException` (test scoped in `pinot-controller`, thus not transitive), every `ControllerTest`-derived test class in `pinot-broker` failed to load and silently ran 0 tests in CI (`SegmentPartitionMetadataManagerTest`, `TimeBoundaryManagerTest`, `SegmentPrunerTest`, `SegmentZkMetadataFetcherTest`, `BrokerRoutingManagerConcurrencyTest`, `RemoteClusterBrokerRoutingManagerTest` — TestNG swallows the `NoClassDefFoundError` and reports `Tests run: 0`). With the dependency restored, all of them run and pass again. ## Testing - New `testReplicationStateTracking`: never-replicated old segment is excluded instead of reducing the fully replicated servers; replica loss after full replication reduces the set; full replica loss excludes + reports the segment without clearing the set; recovery re-includes it; ideal assignment change resets the tracking. - New `testPendingSegmentExpiration`: after the pending window lapses, the segment falls back to reducing the fully replicated servers so its data is still served from the available replica. - Full `pinot-broker` (316 tests) and `pinot-query-planner` (1263 tests) suites pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
