feat: post-pass shard-group reconciliation for asymmetric partial-load matchers#19557
Closed
clintropolis wants to merge 2 commits into
Closed
feat: post-pass shard-group reconciliation for asymmetric partial-load matchers#19557clintropolis wants to merge 2 commits into
clintropolis wants to merge 2 commits into
Conversation
…d matchers changes: * adds `PartialLoadMatcher.emptyMatch` (default null) as an opt-in way to do a zero-content "load" for matchers that can resolve asymmetrically. * `Rule.run` returns a `RuleRunResult` (`OK` constant for most rules). `PartialLoadRule` returns a `ShardGroupFollowup` on a positive match, but only when numCorePartitions > 0. * `RunRules` streams followups in a buffer keyed by (dataSource, interval, version), flushing at iteration boundaries (NEWEST_SEGMENT_FIRST groups segments contiguously by that triple). Flush dispatches emptyMatch loads to unmatched core siblings; siblings not part of the core partition group are skipped. * adds `TimelineLookup.findChunks` which returns a defensive copy via `PartitionHolder.copyWithOnlyVisibleChunks` so iteration is safe outside the timeline lock. * `PartialClusterGroupLoadSpec` allows an empty `clusterGroupIndices` list as the wire form of an empty match.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * </ul> | ||
| */ | ||
| @Nullable | ||
| default MatchResult emptyMatch(DataSegment segment, Map<String, Object> baseLoadSpec) |
| */ | ||
| public interface RuleRunResult | ||
| { | ||
| RuleRunResult OK = new RuleRunResult() |
| * where different partitions of a shard group resolve to different load specs and the broker would otherwise drop | ||
| * the group as incomplete via {@code PartitionHolder.isComplete()}. | ||
| */ | ||
| public record ShardGroupFollowup( |
capistrant
reviewed
Jun 5, 2026
| // Only the core partition group has an atomic-replace completeness requirement on the broker, segments with | ||
| // no core partitions (e.g. append-only streaming) don't need sibling reconciliation. | ||
| if (segment.getShardSpec().getNumCorePartitions() > 0) { | ||
| return new ShardGroupFollowup(segment, matcher, getTieredReplicants()); |
Contributor
There was a problem hiding this comment.
I'm a bit confused about how this behaves when usingonCannotMatch == FULL_LOAD like won't this cause empty loads of other segments that fell through to full load?
Contributor
There was a problem hiding this comment.
will this create core partition set follow ups even when segments who are outside of the core are the only ones who matched? do we want that?
Member
Author
|
^ closed in favor of #19565 |
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
3 participants
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.
Description
This PR adds a post-processing step to
RunRulesso that partial-load matchers that resolve differently across segments of a shard group (e.g.ClusterGroupPartialLoadMatcherover range-partitioned segments) don't leave the broker with an incompletePartitionHolder.Rule.runnow returns a newRuleRunResultobject thatRunRulescan use to 'do stuff', though most implementations today doRuleRunResult.OK. For these asymmetric partial matchers, there is aShardGroupFollowupimplementation ofRuleRunResultthatRunRulescan use to check the siblings of a shard group to perform an 'empty' partial load to ensure that the group is fully available. I am unsure if there is enough of a pattern to makeRunRulesmore generically handleRuleRunResult, i was planning to wait and see if any other use cases pop up before trying to make this handling more generic.Doing an empty load like this seemed cheaper than trying to figure out how to allow the timeline to sometimes allow incomplete groups to appear as complete, since the empty partial load should be cheap for the target historicals.
changes:
PartialLoadMatcher.emptyMatch(default null) as an opt-in way to do a zero-content "load" for matchers that can resolve asymmetrically.Rule.runreturns aRuleRunResult(OKconstant for most rules).PartialLoadRulereturns aShardGroupFollowupon a positive match, but only when numCorePartitions > 0.RunRulesstreams followups in a buffer keyed by (dataSource, interval, version), flushing at iteration boundaries (NEWEST_SEGMENT_FIRST groups segments contiguously by that triple). Flush dispatches emptyMatch loads to unmatched core siblings; siblings not part of the core partition group are skipped.TimelineLookup.findChunkswhich returns a defensive copy viaPartitionHolder.copyWithOnlyVisibleChunksso iteration is safe outside the timeline lock.PartialClusterGroupLoadSpecallows an emptyclusterGroupIndiceslist as the wire form of an empty match.