yujun777 commented on PR #68170: URL: https://github.com/apache/doris/pull/68170#issuecomment-5770525752
**Re: the changes requested in this review** — fixed in 665f86bd5f7. The inline thread has the detail; this is the summary at the review level, since GitHub has no threaded reply for a review body. The pending-partial-baseline case is fixed. `handlePendingIvmBaselineRebuild` now judges `hasUnusableIvmStreamForPartitions(context, baselinePartitions)` — the predicate the partition attempt already uses, over the same set the reset read mode is built from (every non-PCT table of the plan, plus the PCT tables the rebuilt partitions' mappings name) — before it writes its barrier and before it calls the low-level partition refresh. - A fallback-authorized request becomes `[COMPLETE]` and returns without writing a barrier. This is the case `PARTITIONS FALLBACK` hits: it builds `[PARTITIONS, COMPLETE]`, which holds no IVM attempt, so the pre-step is the first place its streams can be judged. COMPLETE reconciles the stream and clears the barrier that is already pending, and a fresh barrier written by a rebuild that never ran would have guarded nothing. - A request that may not fall back fails before the rebuild starts, as it does in the partition attempt, rather than running a rebuild that throws `STREAM_UNSUPPORTED` out of `IvmFullRefreshMTMV`. - `needRefreshPartitions` / `refreshMode` moved below the check, so the fallback path has no field to roll back. On the coverage point: `MTMVTaskTest.testPendingBaselineRebuildChecksTheStreamsItsPartitionsRead` pins exactly the shape you asked for — a non-empty partial barrier plus a missing stream for a table the rebuild reads, plus the strict half. On the previous head it fails with `expected: <[COMPLETE]> but was: <[PARTITIONS, COMPLETE]>`, which is the pre-step's inline rebuild that the assertion catches. I did not add a task-level regression case for it, and I would rather state that than add one that does not reach the path: the state requires a partial refresh that fails *after* its barrier is written, and no debug point injects a failure there. The two on this path cannot — `IvmIncrRefreshManager`'s forced fallback reason decides the incremental attempt before any barrier is written for it, and `TableStreamManager`'s block point parks rather than failing. If you consider a regression case mandatory here, name the injection point you have in mind and I will add it. Verification on this head: `MTMVTaskTest` 49/49, `IvmBaselineRebuildTest` 28/28, `IvmFailureReasonTest` 1/1, `checkstyle:check` clean, and `mtmv_p0/ivm/test_ivm_partitions_fallback_stream_unusable` and `mtmv_p0/ivm/test_ivm_chained_stream_scope` green on a local cluster built from it. -- 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]
