Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
The earlier commits in this stack added a configurable fail-open routing policy. When every enabled replica of a segment is unavailable, the broker can route that segment to a replica that the failure detector has marked unhealthy, instead of reporting the segment as unavailable. The single-stage commit wired this into single-stage selection, with two limits:
BaseInstanceSelector.select()turned fail-open off whenuseMultistageEnginewas set (// TODO: Remove the multi-stage query gate…).MultiStageReplicaGroupSelector, which must keep every segment of an instance partition on one replica group so colocated joins stay correct. A patch can put segment A on RG0 and segment B on RG1. The broker picks this selector from the table's routing config, not the query engine, so the problem affects both single-stage and multi-stage queries on those tables.Summary
Removing the MSE gate
isUseMultistageEnginecheck fromBaseInstanceSelector.select(), so fail-open now applies to multi-stage queries.Fail-open reselection changes (all selectors)
selectFailOpen()now reruns selection over the full per-query segment list in its original order, not just the unavailable subset, and returns that mapping as is. Each segment's candidates:shouldTrackFailOpenCandidatesForHealthySegments(): defaultfalse;truefor Strict andMultiStageReplicaGroupSelector.shouldFailOpenSegmentsWithoutHealthyCandidates(): defaulttrue;falseforMultiStageReplicaGroupSelector.canSkipSegmentForFailOpen(): defaults to the newSegmentStates.isNewSegment();MultiStageReplicaGroupSelectorusesisNewLLCSegment.SegmentStatestracks new segments as part of each snapshot, so skip decisions stay consistent if an assignment refresh races the query.Observability
POOL_SEG_QUERIESis now recorded once per query byBaseInstanceSelector.recordPoolMetrics(), from whichever mapping is actually returned (normal or fail-open). The per-selector recording inBalancedInstanceSelectorandReplicaGroupInstanceSelectoris removed, which fixes double counting during fail-open.FAIL_OPEN_ROUTING_SUCCESSESandFAIL_OPEN_ROUTING_FAILURES.Selector changes
BalancedInstanceSelector: fail-open now goes through the normal selection code with adaptive selection turned off, instead of its own simple round-robin loop. The round-robin sequence continues across fail-open.ReplicaGroupInstanceSelector: fail-open no longer forcesuseFixedReplica=false, so sticky routing now carries through fail-open. Adaptive ranking is still bypassed, and thenumReplicaGroupsToQuerysequence continues across fail-open.StrictReplicaGroupInstanceSelector: opts into tracking fail-open candidates for healthy segments. New segments of an unavailable partition now follow their older siblings onto the same fail-open instance instead of being skipped.MultiStageReplicaGroupSelector:assign(). It first tries the healthy-only map. During fail-open reselection only, if no replica group can serve a partition, it retries with the expanded map that includes fallback candidates. Within the chosen replica group it still prefers healthy instances.select(BrokerRequest, …)override throws if a deferred segment ends up neither assigned nor reported unavailable, so a partition is never silently served with segments missing.InstancePartitionssnapshot from normal selection to the fail-open pass, so both passes use the same partitions. The ThreadLocals are cleared infinallyon every exit path.resolvePreferredReplicaGroup()is shared, so fixed-replica/sticky routing is honored identically on the normal and fail-open paths.Impact on single-stage queries
This PR is not MSE-only. It changes single-stage routing in these ways:
selectFailOpen()reselected only the unavailable segments and merged them into the normal mapping. Now it reselects the full per-query segment list and returns that mapping as is.numReplicaGroupsToQuery, the newly routed unavailable segments shift the replica offset. Either case can move a healthy segment to a different healthy server.ReplicaGroupInstanceSelectorno longer overridesuseFixedReplica=falseduring fail-open. The testtestReplicaGroupFailOpenOverridesFixedReplicaQueryOptionwas replaced bytestReplicaGroupFailOpenPreservesFixedReplicaQueryOption.BalancedInstanceSelectornow uses its normal selection code, with adaptive selection turned off.StrictReplicaGroupInstanceSelectorroutes new sibling segments of an unavailable partition instead of skipping them.POOL_SEG_QUERIESis now recorded centrally on every path.MultiStageReplicaGroupSelectorchanges also apply to single-stage queries.InstanceSelectorFactorypicks it from the table's routing config (MULTI_STAGE_REPLICA_GROUP_SELECTOR_TYPE), so single-stage queries on those tables get the new behavior: deferring a partition to fail-open, the fallback pass, and the new "dropped from the selection" exception.When the fail-open policy is disabled: every reselection change above sits behind
_failOpenPolicy.isEnabled(). The only exception is the pool-metrics refactor, which should be count-equivalent on the healthy path.Motivation
Diagram
flowchart TD A["BaseInstanceSelector.select(brokerRequest, segments)<br/>(SSE and MSE; MSE gate removed)"] --> B["select(segments, ..., failOpenEnabled)<br/>normal pass"] B --> MSE{"MultiStageReplicaGroupSelector?<br/>(table routing config)"} MSE -- yes --> M1["Save InstancePartitions snapshot in ThreadLocal<br/>assign(requireComplete=true)"] M1 --> M2{"Partition assignable<br/>with healthy servers?"} M2 -- yes --> C M2 -- no --> M3{"failOpenEnabled and partition has an<br/>unavailable segment with fail-open candidates?"} M3 -- no --> X1["throw: no replica group can serve partition"] M3 -- yes --> M4["Defer partition's segments<br/>(_segmentsDeferredToFailOpen)"] --> C MSE -- "no (Balanced / ReplicaGroup / Strict)" --> C C{"Unavailable segments<br/>in this request?"} C -- no --> R1["recordPoolMetrics(normal)<br/>return normal mapping"] C -- yes --> D{"Fail-open eligible and enabled?"} D -- no --> R2["recordPoolMetrics(normal)<br/>return normal + unavailable list"] D -- yes --> E["selectFailOpen: rebuild states over FULL segment list<br/>• unavailable → unhealthy candidates<br/>• healthy → normal candidates + fallback (unhealthy) list<br/>• new / candidate-less → skip per hooks"] E --> F["selectForFailOpen()<br/>(adaptive off; sticky/fixed-replica preserved)"] F --> G{"MultiStageReplicaGroupSelector:<br/>assign(requireComplete=false)<br/>healthy map → fallback map as last resort"} G --> H{"All required segments assigned?"} H -- yes --> S["FAIL_OPEN_ROUTING_SUCCESSES++<br/>recordPoolMetrics(fail-open)<br/>return fail-open mapping"] H -- no --> FL["FAIL_OPEN_ROUTING_FAILURES++<br/>rate-limited WARN<br/>recordPoolMetrics(normal)<br/>return normal mapping + unavailable"] S --> V FL --> V{"MultiStageReplicaGroupSelector:<br/>any deferred segment neither<br/>assigned nor unavailable?"} V -- yes --> X2["throw: partition dropped,<br/>no consistent replica group"] V -- no --> OK["return result<br/>(finally: clear ThreadLocals)"]Tests added
MultiStageReplicaGroupSelectorTest(22 new tests)testFailOpenPreservesMultiStageReplicaGroupSelectiontestFailOpenReassignsCompletePartitionToFallbackReplicaGrouptestFailOpenRebuildsReplicaGroupWhenHealthyCandidatesAreSplittestFailOpenRecoversPartitionWithUnavailableSegmentAndHealthySplittestFailOpenOnlyMovesHealthySegmentsOntoUnhealthyServersAsLastResorttestFailOpenLastResortPrefersHealthyReplicaWithinReplicaGrouptestFailOpenRejectsCandidatesSplitAcrossReplicaGroupstestFailOpenStillThrowsWhenSplitPartitionHasNoUnavailableSegmenttestFailOpenThrowsWhenSplitPartitionFailOpenAlsoFailstestFailedFailOpenReturnsPartialNormalAssignmenttestMseFailOpenSkipsCandidateLessNewLlcSegmenttestMseFailOpenDoesNotSkipCandidateLessOldSegmenttestFailOpenSkipsNewSegmentWithoutHealthyCandidatestestFailOpenNewSegmentWithoutHealthyCandidatesDoesNotMoveHealthyPartitiontestMseFailOpenDisabledForUnavailableSegmenttestMseFailOpenNotInvokedForUnavailableSegmentOutsideRequesttestFailOpenReselectionHonorsFixedReplicaStickyRoutingtestMseFailOpenPreservesFixedReplicaQueryOptiontestSelectForFailOpenUsesInstancePartitionsSnapshotFromPrecedingSelecttestThreadLocalsClearedAfterSuccessfulFailOpentestThreadLocalClearedAfterHealthySelectionWithFailOpenDisabledtestThreadLocalClearedAfterSelectionThrowsInstanceSelectorTesttestFailOpenDisabledForMultiStageQuery→testFailOpenEnabledForMultiStageQuerytestReplicaGroupFailOpenOverridesFixedReplicaQueryOption→testReplicaGroupFailOpenPreservesFixedReplicaQueryOptiontestMultiStageFailOpenSkipsCandidateLessNewLLCSegmenttestFailOpenNotInvokedForUnavailableSegmentOutsideRequesttestFailedFailOpenPreservesPartialNormalAssignmenttestBalancedFailOpenContinuesRoundRobinSequencetestBalancedFailOpenDoesNotDoubleCountPoolMetricstestBalancedFailOpenPreservesFixedReplicaQueryOptiontestReplicaGroupFailOpenContinuesNumReplicaGroupsToQuerySequencetestReplicaGroupFailOpenNumReplicaGroupsToQueryGreaterThanReplicastestStrictReplicaGroupFailOpenNeverSplitsAcrossMultipleCandidatestestStrictReplicaGroupFailOpenCoversSiblingNewSegmenttestStrictReplicaGroupFailOpenKeepsHealthySegmentOnHealthyServertestStrictReplicaGroupFailOpenDoesNotMoveHealthyGroupOntoUnhealthyServertestStrictReplicaGroupFailOpenMatchesIdealWhenAllHealthytestStrictReplicaGroupFailOpenIsolatedToHealthyPartitiontestStrictReplicaGroupFailOpenPreservesFixedReplicaQueryOptiontestStrictReplicaGroupFailOpenWithNumReplicaGroupsToQueryReplicaGroupSelectorTesttestReplicaGroupFailOpenBypassesAdaptiveRankingtestBalancedFailOpenBypassesAdaptiveRankingtestStrictReplicaGroupFailOpenBypassesAdaptiveWithoutSplittingPartition