You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "xiangfu0 (via GitHub)" <gi...@apache.org> on 2026/06/08 18:22:37 UTC
[PR] Add best-effort mode for colocated-join routing of transiently-unavailable segments [pinot]
xiangfu0 opened a new pull request, #18706:
URL: https://github.com/apache/pinot/pull/18706
## Problem
Colocated (partition-based) join routing in the multi-stage engine is all-or-nothing. `SegmentPartitionMetadataManager` clears a partition's fully-replicated server set whenever **any** single segment is transiently `OFFLINE` in the ExternalView (segment rebalance, LLC segment commit, server restart). Once that set is empty, `WorkerManager` throws `Failed to find enabled fully replicated server` and fails the **entire** colocated-join query.
These transitions are frequent in production, so colocated-join queries fail intermittently for the duration of the transition window — tuning `pinot.helix.instance.state.maxStateTransitions` does not help, because the routing table is recomputed on every ExternalView change and will observe the momentary `OFFLINE` state regardless.
This is also inconsistent with single-stage routing, which serves the available segments and attaches a non-fatal `BROKER_SEGMENT_UNAVAILABLE` warning rather than failing the query (see `BaseSingleStageBrokerRequestHandler`).
## Change
Adds an **opt-in best-effort mode** that brings colocated-join routing in line with single-stage semantics:
- **`SegmentPartitionMetadataManager`** no longer clears the fully-replicated servers for a transiently-unavailable segment. It records the segment as unavailable (new `PartitionInfo._unavailableSegments`) and keeps routing the partition's available segments. A partition whose segments are **all** unavailable keeps an empty `PartitionInfo` (instead of `null`) so routing consumers fail non-silently rather than dropping it.
- **`WorkerManager`**:
- **strict** (default): a partition with any unavailable segment fails fast with an informative error that names the unavailable segments and points at the option.
- **best-effort**: the available segments are routed, and the unavailable ones are surfaced via the existing `DispatchablePlanMetadata.addUnavailableSegments(...)` channel, which the broker turns into a non-fatal `SERVER_SEGMENT_MISSING` warning plus the `BROKER_RESPONSES_WITH_UNAVAILABLE_SEGMENTS` metric (suppressible via `ignoreMissingSegments`) — exactly like single-stage.
- Controlled by the **`colocatedJoinBestEffort`** query option, which overrides the **`pinot.broker.multistage.colocated.join.best.effort`** broker config (default `false` = original strict behavior). This mirrors the existing `inferPartitionHint` option/config pair.
### Behavior
| Partition state | strict (default) | best-effort |
|---|---|---|
| all segments available | route (unchanged) | route (unchanged) |
| some segments transiently unavailable | **fail** (informative error) | **route available** + non-fatal warning + metric |
| all segments unavailable | fail | fail (non-silent — no server holds the data) |
A fully-unavailable partition still fails in both modes because it cannot be colocated-routed (the leaf stage cannot return an empty partition response), but it fails explicitly rather than silently dropping rows.
## Testing
- `WorkerManagerTest` (new cases): strict-fails-on-unavailable-segment; best-effort via query option; best-effort via broker-config default; multiple-partitions-per-worker (strict + best-effort); fully-unavailable-partition fails non-silently in best-effort mode; hybrid offline+realtime unavailable-segment merge.
- `SegmentPartitionMetadataManagerTest` (new case): a transiently-`OFFLINE` segment preserves the partition's fully-replicated servers and is recorded as unavailable; it returns to the routable list when it comes back online; a fully-unavailable partition keeps an empty `PartitionInfo`.
## Backward compatibility
Default `false` preserves the current strict all-or-nothing behavior — no behavior change unless the option/config is enabled. The new SPI field is added via an additive constructor overload (the existing 2-arg constructor is preserved).
## Notes
Opening as a **draft** for review and CI. A docs/release-note entry for the new query option and broker config will follow.
🤖 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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add best-effort mode for colocated-join routing of transiently-unavailable segments [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 closed pull request #18706: Add best-effort mode for colocated-join routing of transiently-unavailable segments
URL: https://github.com/apache/pinot/pull/18706
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Add best-effort mode for colocated-join routing of transiently-unavailable segments [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18706:
URL: https://github.com/apache/pinot/pull/18706#issuecomment-4652642347
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18706?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 100.00%. Comparing base ([`474efb1`](https://app.codecov.io/gh/apache/pinot/commit/474efb1c1bf881b6cdbc27787e31a4942737ae85?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`4cd5c2a`](https://app.codecov.io/gh/apache/pinot/commit/4cd5c2a0f569f59a07342b1e537dcd108e406b3b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 7 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #18706 +/- ##
==============================================
+ Coverage 64.48% 100.00% +35.51%
+ Complexity 1291 6 -1285
==============================================
Files 3372 3 -3369
Lines 208639 6 -208633
Branches 32590 0 -32590
==============================================
- Hits 134550 6 -134544
+ Misses 63287 0 -63287
+ Partials 10802 0 -10802
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/18706/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/18706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/18706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/18706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/18706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/18706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (+35.51%)` | :arrow_up: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/18706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (+35.51%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/18706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18706/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/18706?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org