Repository navigation
[fix](ivm) Reject an expired stream offset for an IVM read of a duplicate-key table - #69785
Merged
Merged
Conversation
…cate-key table ### What problem does this PR solve? Issue Number: None Related PR: apache#69772 Problem Summary: An IVM materialized view reads each base table through an internal table stream. A duplicate-key base table is read APPEND_ONLY: `OlapTableStreamWrapper.getStreamScanType()` returns APPEND_ONLY for DUP_KEYS, whatever type the stream declares. When the base table's `changelog.ttl_seconds` has expired the stream offset, `NormalizeOlapTableStreamScan.applyChangelogTtl` rejects the read only for MIN_DELTA; for APPEND_ONLY it clips the start to the first retained TSO. For a user stream that clipping is the intended behavior: the stream reads what the changelog still holds. For an IVM refresh it loses data without a trace. The rows written between the offset and the cutoff are never read, the refresh succeeds, the offset moves past them, and no later incremental refresh brings them back. A query on the base table with the MV's shape is answered from the MV by transparent rewrite, so it returns the same wrong result. How to reproduce: create a duplicate-key base table with the changelog enabled and `changelog.ttl_seconds` of a day, build an IVM MV on it (an aggregate MV, or an append-only detail MV), refresh it, insert a row, set `changelog.ttl_seconds` to 1, wait two seconds and run `REFRESH MATERIALIZED VIEW mv INCREMENTAL`. | Refresh after the offset expired | Before | After | | --- | --- | --- | | `REFRESH ... INCREMENTAL` (strict) | SUCCESS, the inserted row is missing from the MV | FAILED, `IvmFallbackReason = CHANGELOG_BROKEN`, the MV keeps its rows | | `REFRESH ... AUTO` | SUCCESS with nothing left to read; the row stays missing | SUCCESS through COMPLETE, `IvmFallbackReason = CHANGELOG_BROKEN`, the MV matches the base table | | the next refresh, with the TTL back to a day | incremental; the row is missing for good | incremental | The fix rejects an expired start for every read of an IVM statement, whatever its scan type. It is the error a MIN_DELTA read already raises, which apache#69772 classifies as `CHANGELOG_BROKEN` and answers with COMPLETE, the only refresh that resets the stream baselines. The condition is the statement (it carries an `IvmRewriteContext`), not the type the stream declares: a user stream on a duplicate-key table also declares MIN_DELTA by default, and it keeps its clipping. An IVM statement never reads a user stream: an MV query that scans a stream is rejected when the MV is created. The snapshot read of a duplicate-key table is bounded by the offset on the base table itself and reads no changelog, so it is unchanged. ### Release note Fix an IVM materialized view over a duplicate-key base table silently missing rows after the table's `changelog.ttl_seconds` expired the stream offset. A refresh that allows fallback now rebuilds the MV with COMPLETE; a strict INCREMENTAL refresh fails with reason `CHANGELOG_BROKEN`. ### Check List (For Author) - Test: Regression test / Unit Test - Regression test: `mtmv_p0/ivm/test_ivm_changelog_offset_expired_dup` (new, nonConcurrent): an aggregate MV and an append-only detail MV over one duplicate-key table, strict and AUTO refreshes after the offset expired and a refresh after the TTL is restored, each MV compared with the base table with MV rewrite off; and a user stream on the table, which still reads past its expired offset. Run on a local shared-nothing cluster, first on a build without this change, where both strict refreshes succeeded without the row and the MVs never caught up; with the change the suite passes, and passes again against the generated `.out`. The whole `mtmv_p0/ivm` directory (114 suites) passes on the same build. Not run on a cloud cluster. - Unit Test: `ExplainTableStreamPlanTest` (two new cases: a user stream read APPEND_ONLY clips its expired offset to the first retained TSO, and the same read in an IVM statement is rejected), run with the IVM unit tests (`org.apache.doris.mtmv.ivm`, `MTMVTaskTest`, `CreateMTMVCommandTest`, `ShowCreateMTMVTest`, `DropMaterializedViewTest`, `ChangelogUtilsTest`): 630 tests pass. - Behavior changed: Yes. An IVM refresh over a duplicate-key table whose stream offset expired falls back to COMPLETE, or fails if it is strict, instead of succeeding without the rows. This includes a partition that was only idle for longer than the TTL before a new write, which a unique-key IVM MV already handles this way; keeping the offsets of caught-up partitions alive is a separate change. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <[email protected]>
…eRefresh alone ### What problem does this PR solve? Issue Number: None Related PR: apache#69772 Problem Summary: A follow-up of the review of apache#69772. Two places in `MTMVTask` were written for the one reason that change produced, `CHANGELOG_BROKEN`, rather than for the property that makes their handling right: the reason's only repair is COMPLETE (`IvmFailureReason.requiresCompleteRefresh()`). - The rebuild an incremental attempt runs before its delta turned a failure into a fallback only when it classified as `CHANGELOG_BROKEN`, and built the fallback with that reason hard-coded. It now filters on `requiresCompleteRefresh()` and passes the classified reason through. That property is what makes the conversion valid there: the rebuild already is the partition refresh, so there is nothing to fall back to short of COMPLETE, and the sync-and-retry the delta has for a missing MV partition does not exist on this path. Any other failure still fails the attempt. No other reason that requires COMPLETE can come out of the rebuild today (the min/max and bitmap guards are only built into delta plans), so the behavior is unchanged. - `handleIvmFallbackResult` had a `CHANGELOG_BROKEN` branch in front of the `requiresCompleteRefresh()` one that returned the same `FALLBACK_TO_COMPLETE` with a differently worded log. It is removed. Why COMPLETE is the only repair of an expired changelog (a partition refresh still reads the streams at their offsets) now sits on `IvmFailureReason.CHANGELOG_BROKEN`, and `requiresCompleteRefresh()` says what the set means. ### Release note None ### Check List (For Author) - Test: Unit Test / Regression test - Unit Test: `MTMVTaskTest` has a new case: a rebuild failure that COMPLETE does not repair (a missing MV partition) still fails an attempt that may fall back. The two expired-offset cases from apache#69772 run through the generalized helper and pass unchanged. With the IVM unit tests: 630 tests pass. - Regression test: `mtmv_p0/ivm/test_ivm_changelog_offset_expired`, which drives both paths, passes unchanged on a local shared-nothing cluster, with the whole `mtmv_p0/ivm` directory. - Behavior changed: No. The fallback for an expired changelog logs the same line as the other reasons that go to COMPLETE. - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <[email protected]>
morningman
requested review from
924060929,
englefly,
morrySnow,
starocean999 and
yujun777
as code owners
October 28, 2026 16:01
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed all seven changed files at 0d4e500f26bfba4918e20386eedb39784065699d. No new actionable findings, and no existing inline P0/P1 findings to carry forward. No additional user review focus was supplied.
Critical checkpoint conclusions:
- Goal and evidence: initialized, expired APPEND_ONLY stream offsets now fail within IVM planning instead of silently clipping. The real incremental and rebuild statement contexts reach the changed predicate. Unit and regression cases cover the intended behavior; they were inspected, not executed.
- Scope and reuse: production changes are focused and reuse the existing expiration helper, failure classifier and
requiresCompleteRefreshdispatch. - Concurrency and locking: no new threads, shared mutable state, locks or lock-order changes; the new boolean and Optional are local to existing statement/task flows.
- Lifecycle and initialization: the same statement context is passed into command execution and planning; existing context cleanup is retained. No new resource ownership or static initialization dependency.
- Configuration: no configuration added; the existing table TTL and statement reference TSO are reused.
- Compatibility: no function signature, persisted format or enum value change; no new rolling-upgrade requirement introduced.
- Thrift/protobuf: no generated-definition changes, so the cross-branch field-id checkpoint is not applicable.
- Parallel paths: examined DUP and non-DUP deltas/snapshots, historical and RESET reads, ordinary user streams, direct incremental queries, dirty-partition rebuilds, COMPLETE, EXPLAIN/dry run and cloud/shared-nothing read-state wiring. RESET and DUP snapshots bypass changelog expiration appropriately; ordinary stream clipping remains unchanged.
- Conditional checks and errors: comments explain why IVM identity is required in addition to MIN_DELTA. Strict requests throw; fallback-enabled requests skip PARTITIONS when COMPLETE is required. Unknown and missing-target-partition rebuild failures remain failures, and the actual classified reason is preserved.
- Test coverage: new planner tests distinguish ordinary clipping from IVM rejection. Rebuild tests retain strict/allowed expiration coverage and add a non-repairable-failure case. The regression suite checks aggregate and detail MVs, ordinary streams, strict failure, AUTO recovery and the next incremental refresh, with both transparent-rewrite paths disabled.
- Expected results: all 20 regression query labels match the output file in order. Ordered rows, aggregate arithmetic, task status and fallback reasons are consistent with the code. Runtime correctness and generated-output provenance were not independently established.
- Observability: existing errors retain start/cutoff TSOs; fallback warnings retain MV/task identifiers, actual reason and detail; task fields expose the reason. No additional observability requirement identified for this gate.
- Transactions and persistence: no journal, replay or transaction protocol change. Failed planning/batches are not credited with successful snapshots or epochs; existing rebuild requirements and held-record publication barriers remain.
- Writes and atomicity: existing insert/overwrite and stream-offset commit paths are preserved. COMPLETE clears prior held records and rebuilds through the established baseline-reset path; a failed COMPLETE still fails the task.
- FE/BE propagation: no new transmitted variable or send path; scan wrappers and pinned cloud visible versions are preserved.
- Performance: one context-presence check plus the existing linear offset transformation; no new RPC, repeated scan or retry loop. COMPLETE recovery intentionally incurs recomputation for correctness.
- Other invariants and limits: no introduced data-visibility, memory-safety or swallowed-error issue substantiated. Missing/null-offset handling is unchanged and was dismissed as a separate pre-existing case, rather than reported as a new defect.
Converged after one review round: both complete-review agents and the separate risk-focused agent returned NO_NEW_VALUABLE_FINDINGS; all changed files and risk items have evidence-backed dispositions, with no unresolved new candidate. Validation was static only: no builds, tests, benchmarks, source edits or submodule initialization.
Review converged after 1 round(s).
Contributor
TPC-H: Total hot run time: 28225 ms |
Contributor
TPC-DS: Total hot run time: 150934 ms |
Contributor
ClickBench: Total hot run time: 24.35 s |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
Author
Steps to reproduceOne duplicate-key table, one IVM MV over it (a plain projection, so an append-only detail MV), one BE. The FE needs #69772 already rejects an expired offset for a unique-key base table, whose internal stream is read CREATE DATABASE IF NOT EXISTS ivm_repro;
USE ivm_repro;
-- Keep the queries on t answered by t itself; step 7 turns this back on.
SET enable_materialized_view_rewrite = false;
DROP MATERIALIZED VIEW IF EXISTS mv;
DROP TABLE IF EXISTS t;
CREATE TABLE t (
k BIGINT NOT NULL,
v BIGINT
)
DUPLICATE KEY(k)
DISTRIBUTED BY HASH(k) BUCKETS 1
PROPERTIES (
"replication_num" = "1",
"changelog.enable" = "true",
"changelog.ttl_seconds" = "86400"
);
INSERT INTO t VALUES (1, 10);
-- 1. Build the MV. Its first refresh is a COMPLETE one and records the offset of the MV's stream on t.
CREATE MATERIALIZED VIEW mv
BUILD IMMEDIATE REFRESH INCREMENTAL ON MANUAL
DISTRIBUTED BY HASH(k) BUCKETS 1
PROPERTIES ("replication_num" = "1")
AS SELECT k, v FROM t;
-- repeat until Status is SUCCESS
SELECT Status, RefreshMode FROM tasks('type'='mv')
WHERE MvDatabaseName = 'ivm_repro' AND MvName = 'mv' ORDER BY CreateTime DESC LIMIT 1;
-- 2. A row written after the offset.
INSERT INTO t VALUES (2, 20);
-- 3. Let the offset, and the row with it, grow older than the TTL before the MV reads them. This stands in
-- for a refresh that comes more than a day after a write, with the default TTL of one day.
ALTER TABLE t SET ("changelog.ttl_seconds" = "1");
SELECT sleep(2);
-- 4. A strict incremental refresh; repeat the task query until the task has finished.
REFRESH MATERIALIZED VIEW mv INCREMENTAL;
SELECT Status, RefreshMode, IvmFallbackReason, ErrorMsg FROM tasks('type'='mv')
WHERE MvDatabaseName = 'ivm_repro' AND MvName = 'mv' ORDER BY CreateTime DESC LIMIT 1;
SELECT * FROM mv ORDER BY k;
-- 5. A refresh that may fall back to COMPLETE; repeat the task query until the task has finished.
REFRESH MATERIALIZED VIEW mv AUTO;
SELECT Status, RefreshMode, IvmFallbackReason, ErrorMsg FROM tasks('type'='mv')
WHERE MvDatabaseName = 'ivm_repro' AND MvName = 'mv' ORDER BY CreateTime DESC LIMIT 1;
-- 6. Compare the MV with the table.
SELECT * FROM mv ORDER BY k;
SELECT * FROM t ORDER BY k;
-- 7. With transparent rewrite on, the same query on t is answered from mv.
SET enable_materialized_view_rewrite = true;
SELECT k, v FROM t ORDER BY k;
EXPLAIN SHAPE PLAN SELECT k, v FROM t ORDER BY k;What it printed on a local shared-nothing cluster:
So without the fix the strict refresh reports success without the row written in step 2, and the AUTO refresh after it does not fall back either. No later incremental refresh brings the row back (the stream offset is now past it), and the query on Variants, all in |
Contributor
FE Regression Coverage ReportIncrement line coverage |
morrySnow
approved these changes
Oct 29, 2026
Merged
3 of 16 tasks
morningman
added a commit
that referenced
this pull request
Oct 29, 2026
…inTableStreamPlanTest (#69809) ### What problem does this PR solve? Issue Number: None Related PR: #69778, #69785 Problem Summary: **In short:** the FE build on master fails, because `ExplainTableStreamPlanTest` now declares the same private method twice. This PR deletes the second copy; nothing else changes. #69778 and #69785 each added the same private helper `normalizeStreamRead(String, long, Optional<IvmRewriteContext>)` to `ExplainTableStreamPlanTest`, at different places in the file. Each PR compiled on its own, and they were merged 41 seconds apart without a rebase onto each other, so master has both copies, and javac rejects the file: ``` ExplainTableStreamPlanTest.java:843: error: method normalizeStreamRead(String,long,Optional<IvmRewriteContext>) is already defined in class ExplainTableStreamPlanTest ``` `build.sh --fe` passes `-DskipTests`, which skips running the tests but still compiles them, so this fails the whole FE build on master and on every PR based on it, not only the FE unit tests. The fix removes the copy #69785 added after its own tests. The two copies are identical, so the tests of both PRs keep calling the same code.
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: #69772
Problem Summary:
In short. An IVM materialized view over a duplicate-key base table silently misses rows once the table's
changelog.ttl_secondshas expired its stream offset: the refresh succeeds, the rows written between the offset and the cutoff are never read, and a base-table query of the MV's shape that transparent rewrite answers from the MV returns the same wrong result. This PR makes every read of an IVM refresh reject an expired offset, whatever its scan type, so the refresh fails withCHANGELOG_BROKENor, when it may fall back, rebuilds the MV with COMPLETE (#69772). It also carries the two follow-ups yujun777 suggested on #69772.Background. An IVM MV reads each base table through an internal table stream. A duplicate-key table is read
APPEND_ONLY:OlapTableStreamWrapper.getStreamScanType()returnsAPPEND_ONLYforDUP_KEYS, whatever type the stream declares.NormalizeOlapTableStreamScan.applyChangelogTtldecides what an expired start means: aMIN_DELTAread is rejected (#69772 turns that intoCHANGELOG_BROKENand COMPLETE), any other read is clipped to the first TSO the changelog retains. The clipping is right for a user stream, which reads the changelog it retains.The problem, and what it cost.
How this PR fixes it.
applyChangelogTtlrejects an expired start for every read of an IVM statement (the statement carries anIvmRewriteContext), whatever the scan type. The condition is the statement, not the type the stream declares: a user stream on a duplicate-key table also declaresMIN_DELTAby default, and it keeps its clipping. An IVM statement never reads a user stream: an MV query that scans a stream is rejected when the MV is created. The snapshot read of a duplicate-key table is bounded by the offset on the base table itself and reads no changelog, so it is unchanged.IvmFailureReason.requiresCompleteRefresh()), and passes that reason through, instead of namingCHANGELOG_BROKEN;CHANGELOG_BROKENbranch ofhandleIvmFallbackResult, which returned the same result as therequiresCompleteRefresh()branch after it, is removed; why only COMPLETE repairs an expired changelog now sits onIvmFailureReason.CHANGELOG_BROKEN.Results (local shared-nothing cluster; a duplicate-key table, a row written after the MV's offset, then
changelog.ttl_secondsset to 1 and two seconds of waiting):REFRESH ... INCREMENTAL(strict)IvmFallbackReason = CHANGELOG_BROKEN, the MV keeps its rowsREFRESH ... AUTOIvmFallbackReason = CHANGELOG_BROKEN, the MV matches the base tableBoth an aggregate MV and an append-only detail MV over the table behave as above.
Classes, and how they call each other.
IvmIncrRefreshMTMV→IvmDeltaRewriteState.createDeltaScan→LogicalOlapTableStreamScan→NormalizeOlapTableStreamScan.makeTableStreamScan→makeIncrementalScanFromChangelog→applyChangelogTtl→ChangelogUtils.effectiveStartTso, which now rejects for an IVM statement.IvmIncrRefreshManager.doRefresh→IvmFailureClassifier.classifyExecutionFailure→CHANGELOG_BROKEN→MTMVTask.handleIvmFallbackResult→ strict: failure with the reason; otherwiseFALLBACK_TO_COMPLETE.MTMVTask.executeIvmAttempt→executePartitionBasedRefresh→ on a failure whose reasonrequiresCompleteRefresh(),handleIvmFallbackResultwith that reason.Not changed: user streams, direct incremental queries (
BindRelation.applyChangelogTtl), the snapshot and reset reads, and anything persisted. A partition with no stream offset is a separate problem, fixed by #69778.Release note
Fix an IVM materialized view over a duplicate-key base table silently missing rows after the table's
changelog.ttl_secondsexpired the stream offset. A refresh that allows fallback now rebuilds the MV with COMPLETE; a strict INCREMENTAL refresh fails with reasonCHANGELOG_BROKEN.Check List (For Author)
mtmv_p0/ivm/test_ivm_changelog_offset_expired_dup(new, nonConcurrent): an aggregate MV and an append-only detail MV over one duplicate-key table, strict and AUTO refreshes after the offset expired and a refresh after the TTL is restored, each MV compared with the base table; and a user stream on the table, which still reads past its expired offset. Local shared-nothing cluster: first on a build without this change, where both strict refreshes succeeded without the row and the MVs never caught up; with the change the suite passes, and passes again against the generated.out. With both commits, the wholemtmv_p0/ivmdirectory passes (114 suites,test_ivm_changelog_offset_expiredof [fix](ivm) Fall back to COMPLETE when an IVM stream offset has expired #69772 included). Not run on a cloud cluster.ExplainTableStreamPlanTest(two new cases: a user stream readAPPEND_ONLYclips its expired offset to the first retained TSO; the same read in an IVM statement is rejected) andMTMVTaskTest(a new case: a rebuild failure that COMPLETE does not repair still fails an attempt that may fall back), run with the IVM unit tests (org.apache.doris.mtmv.ivm,CreateMTMVCommandTest,ShowCreateMTMVTest,DropMaterializedViewTest,ChangelogUtilsTest): 630 tests pass.