hubgeter opened a new pull request, #70508: URL: https://github.com/apache/doris/pull/70508
### Background Iceberg data-file rewriting replaces the source files after reading their logical rows. The distributed rewrite groups share one connector transaction, whose commit validates that no conflicting deletes were added after the read snapshot. ### What problem does this PR solve? Issue Number: None Related PR: #61112 (introduced the explicit rewrite validation anchor) PR Type: Bugfix Problem Summary: A group can pin snapshot S0, then another writer can commit a position delete at S1 before the lazy transaction begin. REWRITE currently anchors validation at S1, so it can accept a replacement built from S0 without detecting that delete. Anchor validation at the group's resolved read snapshot and reject subsequent groups that present a different or unresolved read baseline. Missing, expired, and non-ancestor read snapshots fail explicitly instead of falling back to current metadata. ### Details - **Change map:** Production +29/-13 in one transaction class; tests +325/-34 across the transaction fixture, write-planner fixture, and a new snapshot regression class. No protocol, storage-format, shared SPI, configuration, or BE changes. - **Frequency:** Requires a concurrent row-level delete between the source read pin and the first group begin, or a group with an earlier read pin reaching an already-started shared transaction. - **Root cause:** The REWRITE begin guard ignored the read snapshot already carried by the write context, and the begin-once fast path did not validate later group contexts. - **Why existing tests missed it:** Existing conflict tests began the transaction before committing the delete. They did not cover read-to-begin deletes or different group read baselines. - **Fix:** Require resolved rewrite reads, reuse the existing read-ancestry check, retain the read snapshot for source-file resolution and commit validation, and compare later group reads under the existing begin lock. The SDK transaction still loads current metadata; the validation anchor stays at the read snapshot. - **Other paths and branches:** OVERWRITE keeps its existing validation behavior through the renamed shared helper. `branch-4.2` at `1bb4a49bddc8a505003d2067cc56f0dc3432f23f` already begins its legacy rewrite before group execution and passes one retained snapshot into all groups; this is a source/history assessment, and no backport of the connector change is planned. See the [eager begin and shared snapshot](https://github.com/apache/doris/blob/1bb4a49bddc8a505003d2067cc56f0dc3432f23f/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/rewrite/RewriteDataFileExecutor.java#L73) and [group pin](https://github.com/apache/doris/blob/1bb4a49bddc8a505003d2067cc56f0dc3432f23f/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/rewrite/RewriteGroupTask.java#L314). Groups with different snapshots are deliberately rejected. An unrelated append can therefore cause a conservative failure when different groups observe different read snapshots; an unrelated append with the same read baseline remains allowed. Retry must replan and reread the operation. Passing one operation snapshot to all groups can reduce these conservative failures in a separate change. ### Testing - Before the fix, the two focused regression methods both fail because no exception is thrown: the stale replacement is committed, and the older group read is accepted. - With the fix, the standard `run-fe-ut.sh --run` invocation executes 234 focused tests across `IcebergConnectorTransactionTest` (115), `IcebergWritePlanProviderTest` (108), and `IcebergRewriteSnapshotTest` (11): 0 failures, 0 errors, 0 skipped. The full selected reactor is still compiling subsequent modules. - Iceberg module Checkstyle: 0 violations. Connector import gate and `git diff --check`: passed. - The new tests cover read-to-begin deletes, after-begin deletes, already-visible deletes, unrelated appends, older/newer group reads, unresolved reads, expired snapshots, abandoned history, and empty-read fencing. - These are local component tests with real SDK transactions and simulated file/commit metadata. Physical-file reads, concurrent SQL sessions, cluster behavior, and remote CI have not been validated. ### Notes for Reviewers The existing begin-once lifecycle and lock scope are preserved. Later group validation performs no remote IO under the begin lock. The expected failures from the initial investigation are kept outside this PR; only the product change and maintained tests are included. ### Release note Prevent Iceberg data-file rewrites from accepting stale output after a concurrent row-level delete between read planning and transaction begin. Shared rewrite groups with inconsistent read snapshots now fail explicitly. ### Check List (For Author) - Behavior changed: Yes (see Details). - Does this need documentation: No. - Pre-submission status: Not ready: physical-file and concurrent SQL regression validation is pending. ### AI Disclosure - Tool: Codex. - Generated: Code, tests, and this description. - Human review: Pending maintainer review. -- 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]
