FrankChen021 commented on code in PR #19565:
URL: https://github.com/apache/druid/pull/19565#discussion_r3369495011
##########
server/src/main/java/org/apache/druid/server/coordinator/rules/ClusterGroupPartialLoadMatcher.java:
##########
@@ -59,10 +78,13 @@ public MatchResult match(DataSegment segment, Map<String,
Object> baseLoadSpec)
return null;
}
final List<Integer> resolved = resolveClusterGroupIndices(segment);
- if (resolved.isEmpty()) {
+ final ShardSpec shardSpec = segment.getShardSpec();
+ if (resolved.isEmpty() && shardSpec.getPartitionNum() >=
shardSpec.getNumCorePartitions()) {
+ // No patterns match and this segment isn't part of a core partition
group, so no need for an empty load. Fall
+ // through to the cannot-match handling.
return null;
}
- final String fingerprint = computeFingerprint(resolved);
+ final String fingerprint = resolved.isEmpty() ? EMPTY_LOAD_FINGERPRINT :
computeFingerprint(resolved);
Review Comment:
[P1] Empty matches bypass FALL_THROUGH for fully unmatched groups
Returning a non-null empty MatchResult for every core segment means
PartialLoadRule.appliesTo returns true even when the rule is configured with
onCannotMatch=FALL_THROUGH. If a core shard group has clusterGroups but none of
the segments match the configured patterns, RunRules stops on this partial rule
for every segment, then EmptyDeferringHandler discards all buffered empties
because no positive was seen. Later fallback rules never run, so segments that
previously fell through to a full-load rule can be left with no assignment. The
group-level discard path needs to preserve the configured cannot-match behavior.
##########
processing/src/main/java/org/apache/druid/segment/loading/PartialClusterGroupLoadSpec.java:
##########
@@ -72,10 +71,9 @@ public PartialClusterGroupLoadSpec(
)
{
super(delegate, fingerprint, jsonMapper);
- Preconditions.checkArgument(
- !CollectionUtils.isNullOrEmpty(clusterGroupIndices),
- "clusterGroupIndices must not be null or empty"
- );
+ // An empty index list is used when a cluster-group matcher applies to a
(clustered) segment but no configured
+ // pattern matches its cluster groups. The historical-side partial loader
honors this by loading nothing.
+ Preconditions.checkNotNull(clusterGroupIndices, "clusterGroupIndices");
Review Comment:
[P2] Empty loads still delegate to a full segment load
The constructor now accepts an empty clusterGroupIndices list for the
empty-load sentinel, but PartialClusterGroupLoadSpec still inherits
PartialLoadSpec.loadSegment(), which delegates directly to the inner load spec.
A historical receiving this supposedly empty request will download the full
segment and announce a matching full-fallback profile, so sparse matches can
full-load every empty sibling instead of loading nothing. This needs an
explicit empty-load implementation or another guard before emitting empty
cluster-group load specs.
--
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]
