[fix](fe) Snapshot RF partition metadata during planning - #65919
Merged
morrySnow merged 2 commits intoJul 28, 2026
Merged
Conversation
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Queries can wait in workload group queues after planning but before plan serialization. If a selected RANGE or LIST partition is replaced while queued, runtime-filter monotonicity retains the planned partition ID while OlapScanNode previously rebuilt boundaries from live catalog metadata, omitting the old ID and tripping a BE invariant. Profile explain generation also re-read the replaced partition name. Snapshot RF boundaries and selected partition names during planning so queued queries serialize and report one consistent catalog view.
### Release note
Fix queued runtime-filter queries failing when selected partitions are replaced after planning.
### Check List (For Author)
- Test: Unit Test and Manual test
- FE unit tests for planning-time RF boundary and partition-name snapshots
- Query Queue plus REPLACE PARTITION reproduction on port 9333
- Behavior changed: Yes. Queued queries retain planning-time RF partition metadata and explain names.
- Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
BiteTheDDDDt
marked this pull request as ready for review
July 22, 2026 12:25
BiteTheDDDDt
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
July 22, 2026 12:25
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
TPC-H: Total hot run time: 29651 ms |
Contributor
TPC-DS: Total hot run time: 177650 ms |
Contributor
ClickBench: Total hot run time: 25.21 s |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Found one correctness issue that should be fixed before merge: RF partition-pruning enablement is read once during translation and again after workload-queue admission, but workload policy can mutate the live session variable between those points. A false-to-true transition now leaves a pruning-marked scan without a boundary snapshot and makes FE serialization fail.
Critical checkpoints:
- Goal and data correctness: the planning-time snapshot fixes queued
REPLACE PARTITIONfor ordinary locked OLAP scans, but the mutable enablement path means the implementation is not complete yet. - Scope and clarity: the three-file change is focused and reuses the existing boundary builder; no unrelated changes were found.
- Concurrency and lifecycle: table metadata is protected through normal Nereids translation, selected names are refreshed on all production ID-mutation paths, and retries/short-circuit reuse are safe. The asynchronous workload-policy writer is the unresolved lifecycle race described inline.
- Configuration and compatibility: no new config, persistence format, or FE-BE protocol field is introduced. The issue comes from treating the existing dynamic session value as an immutable plan decision.
- Parallel paths: grouped and single RF translation both use the affected helper. RANGE/LIST/default/null/multi-column boundary alignment is otherwise preserved; CTE and TVF targets do not create an additional case under current RF eligibility gates.
- Tests: the added RANGE helper test validates retained IDs/names across replacement, but it bypasses the real translator/session gate/
toThrift()lifecycle and cannot catch the accepted race. Please add the targeted false-to-true test requested inline. Per the review-runner contract I did not run local builds or tests; CI compile and CheckStyle were green during review, while the author reports focused FE UT and a manual queued-query reproduction. - Persistence, data writes, and observability: not applicable to this planner-only metadata-lifetime change; existing RF pruning counters remain available.
No additional user-provided focus points were supplied.
| "partition-pruning runtime filter target must be an OlapScanNode"); | ||
| runtimeFilter.markTargetCanPrunePartitions(scanNode.getId()); | ||
| ConnectContext rfPruneCtx = ConnectContext.get(); | ||
| if (rfPruneCtx != null |
Contributor
There was a problem hiding this comment.
enable_runtime_filter_partition_prune is not stable across these two checks. The workload-policy thread can call WorkloadActionSetSessionVar on a live query's SessionVariable; if this flag is false here, the target is still marked as partition-prunable but no snapshot is stored. If the policy flips it to true while the query waits in the workload queue, OlapScanNode.toThrift() re-reads true, sees the marked RF, and calls setPartitionBoundariesForRuntimeFilter(), which now fails because the snapshot is null. (Before this change, that serialization-time path built the boundaries instead.) Please capture one immutable per-plan enablement decision, or snapshot every marked target regardless of the later-mutable flag, and cover the false-to-true transition through the real translator/toThrift() path in a test.
Contributor
FE UT Coverage ReportIncrement line coverage |
morrySnow
requested changes
Jul 23, 2026
morrySnow
left a comment
morrySnow
left a comment
Contributor
There was a problem hiding this comment.
Overall Assessment
This PR correctly fixes a real TOCTOU bug: when a query is queued in a Workload Group, selected RANGE/LIST partitions can be replaced before Thrift serialization, causing stale runtime-filter partition boundary metadata to be sent to BE. The fix snapshots both partition boundaries and partition names at planning time, which is the right approach.
The refactoring (extracting setPartitionPruningMetadata helper, splitting setPartitionBoundaries into buildPartitionBoundariesForRuntimeFilter + setPartitionBoundariesForRuntimeFilter) is clean and reduces duplication.
The unit test is well-structured: it sets up a RANGE-partitioned scan, snapshots, mutates the live catalog to simulate REPLACE PARTITION, and verifies serialization and explain output still use the planned data.
Issues requiring changes
1. Session variable TOCTOU between planning and serialization
The snapshot in setPartitionPruningMetadata() is gated on isEnableRuntimeFilterPartitionPrune() (line 394 of the new code), and the consumption in toThrift() → setPartitionBoundariesForRuntimeFilter() is also gated on the same session variable. If the session variable were to change from false during planning to true during serialization, markTargetCanPrunePartitions() would have been called (so hasRfDrivingPartitionPruning() returns true), but no snapshot was taken — the Preconditions.checkNotNull would fail.
While session variables don't typically change mid-query, the coupling is fragile. Consider either (a) also guarding markTargetCanPrunePartitions() by the session variable, or (b) making setPartitionBoundariesForRuntimeFilter() fall back to building from the live catalog when the snapshot is null (with a warning log).
2. Minor: format specifier %s for Long
In getSelectedPartitionNamesForExplain(), the Preconditions.checkNotNull error message uses %s for a Long parameter id. The %s formatter works (auto-boxing → toString()), but %d is more idiomatic for numeric types.
### What problem does this PR solve? Issue Number: None Related PR: apache#65919 Problem Summary: Workload policies can mutate enable_runtime_filter_partition_prune while a planned query waits in a workload queue. The initial fix marked partition-pruning runtime-filter targets independently of the setting but only snapshotted boundaries when the setting was enabled, so a false-to-true change could make Thrift serialization consume a missing snapshot. Snapshot every marked target during planning so dynamic enablement safely uses the same catalog view. Add a real Nereids translator-to-Thrift unit test for the false-to-true transition, verify repeated snapshots retain the first catalog view, and use numeric formatting for partition IDs. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.glue.translator.PhysicalPlanTranslatorTest#testRfPartitionPruneSnapshotSurvivesEnablementChange,org.apache.doris.planner.OlapScanNodeTest#testRuntimeFilterPartitionBoundariesUsePlanningSnapshot,org.apache.doris.nereids.glue.translator.RuntimeFilterPartitionPruneClassifierTest - ./run-fe-ut.sh --coverage --run org.apache.doris.nereids.glue.translator.PhysicalPlanTranslatorTest#testRfPartitionPruneSnapshotSurvivesEnablementChange,org.apache.doris.planner.OlapScanNodeTest#testRuntimeFilterPartitionBoundariesUsePlanningSnapshot,org.apache.doris.nereids.glue.translator.RuntimeFilterPartitionPruneClassifierTest - ./build.sh --fe - Behavior changed: Yes. Eligible RF targets retain planning-time boundary snapshots even when partition pruning is disabled until a workload policy enables it. - Does this need documentation: No
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
TPC-H: Total hot run time: 29507 ms |
Contributor
There was a problem hiding this comment.
Review outcome: requesting changes for one avoidable FE planning regression, described inline. The original queued-REPLACE PARTITION correctness failure and the previously reported false-to-true session-variable race are otherwise addressed on the current head.
Critical checkpoints:
- Goal, data correctness, and tests: planning-time snapshots now keep selected IDs, boundaries, monotonicity, scan ranges, and explain names on one catalog view. The replacement test and the translator-to-Thrift false-to-true test cover the two reported lifecycle failures. A one-selected-partition LIST case is still needed with the inline fix.
- Scope and clarity: the four-file FE change is focused; grouped and single RF translation share one helper, and the boundary builder/refill split is clear.
- Concurrency and lifecycle: Nereids holds table read locks through physical translation, so classification and snapshotting are aligned. All production selected-partition-ID writers snapshot names before explain. The current unconditional snapshot keeps later workload-policy enablement safe.
- Configuration and compatibility: no new configuration, persistence format, function symbol, storage format, or FE-BE protocol field is introduced. The existing dynamic session setting is handled safely across true/false/no-context transitions; optional metadata mismatches only disable RF partition pruning.
- Parallel paths: grouped RFs with different targets on one scan skip ambiguous scan-ID metadata; repeated RFs safely share one scan snapshot; CTE/TVF and short-circuit point-query paths do not create an additional RF snapshot case.
- Performance: the remaining issue is the guaranteed-unused O(LIST values) snapshot for a scan with fewer than two selected partitions. RANGE/LIST/default/null/multi-column handling is otherwise preserved.
- Persistence, data writes, and observability: not applicable to this planner-only metadata-lifetime change; existing RF pruning counters remain available.
- Validation: per the review-runner contract, I did not run local builds or tests. At the final check, GitHub CheckStyle, license, secret, dependency, and macOS BE checks had passed; TeamCity compile, FE UT, and performance checks were still pending. The author reports focused FE unit tests and manual queued-query validation.
- User focus: no additional user-provided focus points were supplied.
Contributor
TPC-DS: Total hot run time: 178714 ms |
Contributor
ClickBench: Total hot run time: 24.94 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
morrySnow
approved these changes
Jul 28, 2026
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.
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
A query can finish planning and then wait in a Workload Group queue before its plan is serialized. If a selected RANGE or LIST partition is replaced during that wait, runtime-filter target metadata still references the planned partition ID, while
OlapScanNodepreviously rebuilt runtime-filter partition boundaries from the current catalog during Thrift serialization. The removed partition ID was therefore absent from the boundaries sent to BE, which violated the runtime-filter partition-pruning invariant and failed the query withboundary_partition_ids.contains(partition_id).This change snapshots runtime-filter partition boundaries during planning, from the same catalog view used to select partitions and build scan ranges. It also snapshots selected partition names so profile/explain generation does not look up a partition that was replaced after planning. No BE or Thrift protocol changes are required.
The FE unit test snapshots a RANGE-partitioned scan, mutates the live
PartitionInfoand partition map to simulateREPLACE PARTITION, and verifies that serialization and explain output continue to use the planned partition IDs and names.Release note
Fix queued runtime-filter queries failing when selected partitions are replaced after planning.
Check List (For Author)
Test
./run-fe-ut.sh --run org.apache.doris.planner.OlapScanNodeTest#testRuntimeFilterPartitionBoundariesUsePlanningSnapshot,org.apache.doris.nereids.glue.translator.RuntimeFilterPartitionPruneClassifierTest./build.sh --fe1 | 900), a new query returned the replacement data (1 | 1900), and BE remained alive.Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)