yujun777 commented on PR #68180: URL: https://github.com/apache/doris/pull/68180#issuecomment-5770757701
**Re: the changes requested in this review** — all three items are fixed in 57a1b3d71dc. Each thread has the detail; this is the summary at the review level. - **Aliased recovery (the new P1)**: the membership check now requires the *incarnation*, not the name — each changed base partition must resolve to the live partition the caller supplied, by id, or the whole MV is rebuilt. Only RECOVER can supply an incarnation that is not live, which is why DROP, TRUNCATE and strict REPLACE are unaffected. - **The property-generation race (the re-raised P1)**: this head is affected. The lineage mapping that answered it at 7ad093ee957 is gone, so the selection is windowed again and reads the limit from the very properties the mapping is built from. The limit is now read on both sides of the mapping construction, and a limit in effect on either side makes the mapping untrustworthy. I did not generation-guard the sync limit — that would change non-IVM MVs and belongs in a change of its own, as I said in the earlier round. What this changes is the direction of the failure: from trusting an incomplete mapping, which loses rows permanently, to rebuilding the whole MV. - **The rollover-sensitive bounds (the new P2)**: both sync-window tests now end their recent partition two years out, so no year boundary between building the DDL and marking the change can land its upper bound on the cutoff. Two things I would rather state than leave to be inferred: 1. The window guard is not the only net for an all-filtered mapping. When the window filters out every base partition of the PCT table, the mapping drops the table entirely and the answer comes from the earlier "base table is not described by the mapping" branch. `testChangedPartitionOutsideTheSyncWindowRebuildsTheWholeMv` passes through *that* branch rather than the guard; the guard's shape is the mixed one. Both answer COMPLETE, so this is about which path the coverage actually exercises, not about correctness. 2. The interleaving behind the property race is not staged in a test. The mapping is built with no injection point between the two reads, so the unit test pins that both reads happen — a single read fails it — rather than a racy outcome. If a regression case is required there, name the injection point and I will add it. Verification on this head: `IvmBaselineRebuildTest` 36/36, `MTMVTest` 23/23, `MetaLockUtilsTest` 6/6, `MTMVRefreshSnapshotTest` 2/2, `checkstyle:check` clean; `mtmv_p0/ivm` test_ivm_baseline_marker_scope, test_ivm_partition_baseline_rebuild, test_ivm_partition_baseline_rebuild_dup_keys, test_ivm_drop_referenced_column_baseline_rebuild, test_ivm_partition_drop_live_delta, test_ivm_replace_table_reconcile, test_ivm_replace_stream_cleanup and test_ivm_drop_column_fallback_reason 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]
