JanKaul commented on code in PR #2620: URL: https://github.com/apache/iceberg-rust/pull/2620#discussion_r4014360682
########## docs/rfcs/0003_stateful_transaction.md: ########## @@ -0,0 +1,418 @@ +<!-- + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. +--> + +# RFC: Stateful Transactions and Snapshot Production + +**Status:** Draft +**Target:** Apache Iceberg Rust (`iceberg-rust`) + +## 1. Motivation and Scope + +Replaying transaction actions after a catalog conflict is sufficient for simple +metadata updates, but snapshot-producing actions benefit from retaining work +across attempts. FastAppend and merging operations such as RowDelta need stable +execution identity, reusable metadata processing, and ownership of generated +files that may require cleanup. + +This RFC establishes a stateful action model and a shared foundation for snapshot +production. The transaction owns catalog refresh and replay; each action execution +owns retry-persistent state; snapshot-producing actions use persistent producers +as that state. + +The proposal fixes lifetimes, ownership, the core action interface, and the +responsibilities of snapshot producers. It does not prescribe cache layouts, +conflict-validation predicates, or the complete semantics of each operation. +Those can be implemented incrementally within these boundaries. + +## 2. Transaction and Action State Model + +### 2.1 Three Lifetimes + +| Lifetime | Responsibility | +| --- | --- | +| Transaction | Catalog refresh, retry policy, action ordering, replay, and the terminal result | +| Action execution | Immutable operation intent and exclusively owned retry-persistent state | +| Attempt | Execution against the current transaction-local table and construction of that attempt's result | + +An action's intent does not change during replay. Its state survives attempts of +one logical execution, while values derived from a particular base are local to +an attempt or reused only under an appropriate validity check. + +### 2.2 Stateful Action Interface + +Each action declares an associated state type. The following interface captures +the lifecycle; the heterogeneous storage adapters are implementation details. + +```rust +#[async_trait] +pub(crate) trait TransactionAction: Send + Sync + 'static { + type State: Send + Sync + 'static; + + /// Fresh state for one logical execution; infallible and table-independent. + fn new_state(&self) -> Self::State; + + /// One attempt against the current transaction-local table. + async fn commit( + &self, + state: &mut Self::State, + table: &Table, + ) -> Result<ActionCommit>; Review Comment: Should the model pin down the two inputs every predicate shares before the interface settles? In Java each `MergingSnapshotProducer` validation is anchored on `startingSnapshotId` and checks the range (start, currentParent]. Two questions: 1. Starting snapshot + isolation look like immutable intent rather than State — they're caller-set and can even predate the transaction (the snapshot a read ran against), so they can't be re-derived from the current table. Does `commit(&self, …)` intend `&self` to carry them, and should the RFC say so? If the lower bound is instead re-derived per attempt, doesn't the range collapse and validation silently pass? 2. The added data/delete-file index over (start, parent] seems like a natural fit for State: only parent advances across retries, so the index can be reused and extended (S→C1→C2) instead of re-read from start each attempt — something Java doesn't do. Is that the intended use of persistent producer state, and worth citing in §4/§5 as concrete motivation? (Only the index is reusable; the verdict re-runs each attempt — which may also sharpen the §5.1 row on line 236.) -- 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]
