feat: improved partial load rule behavior by clintropolis · Pull Request #19903 · apache/druid

clintropolis · 2026-08-06T12:02:52Z

Description

This PR is a follow-up to #19791 to improve the behaviors possible with partial load rules/matchers, adding 2 new LoadSpec and CannotMatchBehavior implementations.

changes:

adds CannotMatchBehavior.BASE_LOAD to indicate that when partial rules interval applies but fails to match it will load the 'base table' (no projections) with newly added PartialBaseTableLoadSpec
adds CannotMatchBehavior.LOAD_ON_DEMAND which has taken over the previous behavior that was for FULL_LOAD (issues the load spec directly with no 'partial' wrapper). Since partial loads only apply when the historical is in 'virtual storage' mode, a regular load rule is a 'weak' load on demand load.
CannotMatchBehavior.FULL_LOAD now issues a PartialFullSegmentLoadSpec which loads everything in the segment, to make it honest about what it does.
PartialProjectionLoadSpec on failure to match will now always pick a PartialBaseTableLoadSpec since projections can always be recomputed from the base table so that historicals are still eagerly loading some data that can satisfy user queries instead of falling back to the CannotMatchBehavior

FrankChen021

Severity	Findings
P0	0
P1	3
P2	0
P3	0
Total	3

Reviewed 15 of 15 changed files. Projection fallback compatibility, partial-to-on-demand reconciliation, and unsupported FULL_LOAD backends need correction.

This is an automated review by Codex GPT-5.6-Sol

FrankChen021 · 2026-08-06T12:36:32Z

    final List<String> resolved = resolveProjectionNames(segment);
    if (resolved.isEmpty()) {
-      return null;
+      return new MatchResult(


[P1] Respect onCannotMatch when projections are absent

Returning a base-table result here means PartialLoadRule never consults onCannotMatch when no configured projection exists. Existing rules using onCannotMatch: fallThrough followed by another load or drop rule now stop the cascade and base-load the segment instead, silently changing tier placement and retention behavior. Return null for an empty resolution and let the newly added BASE_LOAD behavior opt into this fallback, or add a separate explicit matcher option.

I thought this behavior made more sense for projection matching because a partial load rule is an intent to pre-warm data loading on historicals so that queries can be fast and projections can always be re-computed from the 'base' table if the desired projections are not present in the segment, so this change ensures that at least some user data is loaded that can satisfy user queries, and behaves better when used with a CompositePartialLoadMatcher, since a missing projection will not fail the whole match (this discussion #19791 (comment)).

FrankChen021 · 2026-08-06T12:36:33Z

+    // Matcher does not apply, but the rule still does — FALL_THROUGH would have made appliesTo return false, so run
+    // wouldn't have been invoked. How much of the segment to make resident is onCannotMatch's call.
+    switch (onCannotMatch) {
+      case LOAD_ON_DEMAND -> handler.replicateSegment(segment, getTieredReplicants());


[P1] Reconcile partial profiles before going on-demand

When a replica currently announces a partialBaseTable or partialFullSegment profile and the rule changes to LOAD_ON_DEMAND, this enters the fingerprint-blind regular replication path. StrategicSegmentAssigner.updateReplicasInTier exits as soon as the replica count is satisfied, so it never sends the wrapper-free load request that makes SegmentLocalCacheManager release the old rule holds. The old bundles therefore remain pinned and the stale profile persists indefinitely. Reconcile partial-to-regular transitions explicitly, or represent on-demand state with a profile the partial reconciler can swap to.

~~> Reconcile partial-to-regular transitions explicitly, or represent on-demand state with a profile the partial reconciler can swap to.~~

~~The latter sounds better to me.~~

edit: response in thread clarified handling of this

that is fixed by #19884

FrankChen021 · 2026-08-06T12:36:33Z

+          PartialBaseTableLoadSpec.wireForm(segment.getLoadSpec(), PartialBaseTableLoadSpec.FINGERPRINT),
+          PartialBaseTableLoadSpec.FINGERPRINT
+      );
+      case FULL_LOAD -> replicateWholly(


[P1] Honor FULL_LOAD for non-rangeable segments

On a backend that cannot open a range reader, such as zipped or legacy deep storage, loadPartial clears any prior rule and returns the plain segment without downloading or pinning anything. FULL_LOAD therefore degrades to weak on-demand loading instead of making every bundle resident; because the plain announcement has no requested fingerprint, the coordinator continues treating the replica as stale and can requeue the same request each run. Add an eager full-download-and-pin fallback for this wrapper, or reject or advertise the unsupported state explicitly rather than promising FULL_LOAD.

I don't think we actually end up with re-queue constantly in this case. wont we just lazily load the full segment, but still have announced it with the full fingerprint and full size accounting?

one alternative I've thought about for handling non range readable segments is a metric emitted with datasource, fingerprint dims when end up loading a situation like this. I'm not sure how difficult having a full load for non ranageable segments is in this case, so I figured a metric alerting an operator to this could be more straightforward to implement

yea, this is a bit of a rough edge but I don't think i'm going to change it in this PR and since this functionality is still undocumented and a bit experimental i think its ok for now. Historicals currently lack the machinery to do sticky eagerly loads of non-rangeable segments since they currently are stored in different cache entries (PartialSegmentMetadataCacheEntry/PartialSegmentBundleCacheEntry vs CompleteSegmentCacheEntry for non-rangeables), so this would take some work to do to add that weak hold machinery to prevent eviction and honor the rule, or, they would need to be stored as 'static' entries in the cache and managed that way, both of which are not directly related to the changes in this PR.

capistrant

commits got pushed mid review 😛 posting and I will mark anything stale as resolved

capistrant · 2026-08-06T18:04:26Z

+    // Matcher does not apply, but the rule still does — FALL_THROUGH would have made appliesTo return false, so run
+    // wouldn't have been invoked. How much of the segment to make resident is onCannotMatch's call.
+    switch (onCannotMatch) {
+      case LOAD_ON_DEMAND -> handler.replicateSegment(segment, getTieredReplicants());


~~> Reconcile partial-to-regular transitions explicitly, or represent on-demand state with a profile the partial reconciler can swap to.~~

~~The latter sounds better to me.~~

edit: response in thread clarified handling of this

capistrant · 2026-08-06T18:28:20Z

+          PartialBaseTableLoadSpec.wireForm(segment.getLoadSpec(), PartialBaseTableLoadSpec.FINGERPRINT),
+          PartialBaseTableLoadSpec.FINGERPRINT
+      );
+      case FULL_LOAD -> replicateWholly(


I don't think we actually end up with re-queue constantly in this case. wont we just lazily load the full segment, but still have announced it with the full fingerprint and full size accounting?

one alternative I've thought about for handling non range readable segments is a metric emitted with datasource, fingerprint dims when end up loading a situation like this. I'm not sure how difficult having a full load for non ranageable segments is in this case, so I figured a metric alerting an operator to this could be more straightforward to implement

capistrant · 2026-08-06T18:49:07Z

+ * <b>Projection matchers always apply.</b> When none of the configured projections are present on a segment, the
+ * matcher resolves to a {@link PartialBaseTableLoadSpec} (every row, no projections) instead of going opaque. A
+ * projection is precomputation that is always recoverable from the base table, so the base table is a correct
+ * substitute for one the segment doesn't carry, and it is strictly less data than every bundle on the segment. This
+ * is the ordinary state of affairs mid-rollout, when a new projection is being reindexed in and only some segments
+ * carry it yet.


I think I understand the why for this behavior. queries that would use the projection can still run on the data even if the projection doesn't exist. but what about cases like:

bad rules that misdefine a projection name and end up loading the base table

composites where maybe a few cluster groups + projection were spec'd in the rule but with the base table you end up with essentially the whole segment if the projection was missing

Either of those things is technically correct under documentation. But is it rather something an operator should specify in the projection rule rather than be boxed into it? maybe it even defaults like this so an operator has to consciously say if there isn't projection match then I understand the rule will do onCannotMatch behavior. and as I type this I ask - is this idea any better? 🫠

#19903 (comment) kind of responds to this comment

like could we have projection rules carry a no matched projections behavior desire of load base table on demand or base table eagerly I guess is what I'm noodling on.

but maybe I'm overcomplicating it and as long as this is defined behavior that it is a choice an operator takes when they use projection based partial rules

I think I take this all back after considering that it is going to be clearly defined behavior. projections rules will be created because the operator expects the projection to exist (at least eventually in the case of adding with compaction), and they don't want to just not see data because it wasn't created yet when the alternative is a non-projection query that gives back good results, albeit more slowly.

capistrant

Responses to Frank make sense to me. My comments either group under those discussions or I have came to no longer think they are issues that need to be addressed. One nit on an annotation other than that I think this is a good step improvement

capistrant · 2026-08-06T19:41:37Z

+   * them are present on {@code segment}.
+   */
  @Override
  @Nullable


nit: this impl is no longer @Nullable

…n-matcher-improvement

feat: improved partial load rule behavior

fa99991

github-actions Bot added the Area - Segment Format and Ser/De label Aug 6, 2026

FrankChen021 reviewed Aug 6, 2026

View reviewed changes

unused import

0757351

capistrant reviewed Aug 6, 2026

View reviewed changes

capistrant approved these changes Aug 6, 2026

View reviewed changes

clintropolis added 2 commits August 6, 2026 14:04

Merge remote-tracking branch 'upstream/master' into partial-projectio…

5310629

…n-matcher-improvement

remove nullable annotation

8ee652d

clintropolis closed this Aug 6, 2026

clintropolis reopened this Aug 6, 2026

clintropolis added 2 commits August 6, 2026 19:02

Merge remote-tracking branch 'upstream/master' into partial-projectio…

9db9d4b

…n-matcher-improvement

unused import

f0d4561

Uh oh!

Conversation

clintropolis commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

FrankChen021 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

capistrant Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

capistrant left a comment

Choose a reason for hiding this comment

Uh oh!

capistrant Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

capistrant Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

capistrant left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

clintropolis commented Aug 6, 2026 •

edited

Loading

capistrant Aug 6, 2026 •

edited

Loading

capistrant Aug 6, 2026 •

edited

Loading

capistrant Aug 6, 2026 •

edited

Loading