Transaction commits against a base it never validated, so an external writer cannot make a commit conditional
Los mantenedores suelen responder en 1 día
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Aptitud para principiantes
- 35/100
Línea de trabajo
Empieza leyendo Transaction::do_commit y la visibilidad de TransactionAction en transaction/action.rs; después, inspecciona TableCommit en catalog/mod.rs. Ejecuta la reproducción independiente enlazada para confirmar el comportamiento con una base obsoleta. La finalización depende de la dirección elegida por los maintainers: el commit condicional debe poder expresarse y rechazar la transacción obsoleta, o se debe documentar la limitación.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
What happens
A Transaction built from table state X commits successfully after the table
has moved to Y. Transaction::do_commit reloads the table, sees the base is
stale, replaces it, and re-derives the action's TableRequirements from Y. The
state the caller inspected before building the transaction never constrains the
commit.
This is correct for an unconditional append, which commutes. It is not correct
for a commit that carries a precondition, and there is currently no way for an
external crate to express one.
Reproduction
No arrow, no parquet, no data files. Depends only on iceberg, tokio and
tempfile. Two transactions carrying snapshot summary properties are enough,
because the issue is in how Transaction picks its base rather than in what the
action writes.
// Writer A loads the table and correctly observes that epoch 1 is absent.
let a_base = catalog.load_table(&ident).await?;
assert!(epochs(&a_base).is_empty());
// Writer B does the same and commits first.
let b_base = catalog.load_table(&ident).await?;
let tx = Transaction::new(&b_base);
let action = tx.fast_append()
.set_snapshot_properties(HashMap::from([("example.epoch".to_string(), "1".to_string())]));
action.apply(tx)?.commit(&catalog).await?;
// A commits the transaction it built from `a_base`, which is now stale.
// Expected: an error, so A can re-read and discover it lost.
// Actual: Ok.
let tx = Transaction::new(&a_base);
let action = tx.fast_append()
.set_snapshot_properties(HashMap::from([("example.epoch".to_string(), "1".to_string())]));
action.apply(tx)?.commit(&catalog).await?;
assert_eq!(epochs(&catalog.load_table(&ident).await?), vec!["1", "1"]);
B committed epoch 1
A commit result: Ok
epochs recorded in the table: ["1", "1"]
The table is created with commit.retry.num-retries = 0, to show the retry loop
is not the cause. do_commit rebases at the top of every call, including the
first, before any failure has occurred.
Full runnable crate: https://github.com/AndreaBozzo/iceberg-stale-base-repro
Why it matters
The concrete case is idempotent writes from an external engine. A writer that
crashes between committing and observing success must be able to retry without
producing a second copy, which needs a commit conditional on "this identifier
has not already been applied". Delta expresses this with its txn action;
Iceberg's specification supports it through requirements and the atomic
metadata-pointer swap. In iceberg-rust it currently cannot be expressed.
Why there is no workaround
TransactionActionispub(crate)(transaction/action.rs:37), so an
external crate cannot define an action whosecommit(&table)re-validates
against the refreshed base. That method is called with exactly the right table
bydo_commit; it just is not reachable.TableCommit's builder ispub(crate)too (catalog/mod.rs:375), so a caller
cannot construct a commit carrying its own
TableRequirement::RefSnapshotIdMatch, even though
Catalog::update_table(TableCommit)is public. The doc comment is explicit
thatTransactionis the intended path.- Checking before
Transaction::commitdoes not help, because the rebase
discards the base the check was made against.
The Java library has this hook
SnapshotProducer.validate(TableMetadata currentMetadata, Snapshot snapshot) is
protected (core/src/main/java/org/apache/iceberg/SnapshotProducer.java:281).
apply() calls it (line 372), and the commit loop calls apply() on every retry
attempt (lines 485-497), so an operation re-validates against the refreshed base
each time. BaseRowDelta, BaseRewriteFiles and StreamingDelete all
override it.
iceberg-rust has the structurally identical hook in
TransactionAction::commit(&Table), invoked by do_commit against the refreshed
table. The difference is only that Java's is subclassable and Rust's is
pub(crate).
Possible directions
- Make
TransactionActionpublic, so a caller can define an action that
validates against the refreshed base inside the existing loop. Smallest
change, and it matches the Java model. - Allow a caller-supplied
TableRequirementon aTransaction, so the base a
caller checked can be pinned across the rebase. - If the rebase is intended to be unconditional, document that a
Transaction
carries no guarantee about the base it was constructed from, so callers do
not build preconditions on it.
Happy to open a PR for (1) if that is the direction maintainers prefer.
Checked against main
All three code references above are current on main, not only on the 0.10.1
release. I searched the tracker and did not find this covered; the nearest
neighbours looked like #964 (commit retries, closed — its step 2, "store the
update actions and reapply them to the table when the commit fails", is where
the rebase comes from) and #1939 / #3019, which are other requests to attach
semantics to a commit atomically.
- Lenguaje dominante
- Rust
- Estrellas
- 1.4k
- Forks
- 574
- Merge medio
- 1 d 19 h
- PR fusionados (30 d)
- 66
Preparar el entorno
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de apache/iceberg-rust
-
datafusion
Dificultad 2/5 1-3 horas Aptitud para principiantes 76/100
apache/iceberg-rust#3297 ·
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
apache/iceberg-rust#3285 · 1 reacción ·
Los mantenedores suelen responder en 1 día
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 86/100
apache/iceberg-rust#3280 ·
Los mantenedores suelen responder en 1 día
-
enhancement
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
apache/iceberg-rust#3234 · 2 reacciones ·
Los mantenedores suelen responder en 1 día
-
bug
Dificultad 2/5 1-3 horas Aptitud para principiantes 72/100
apache/iceberg-rust#3229 ·
Los mantenedores suelen responder en 1 día
Todos los issues de apache/iceberg-rust
Issues similares
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
aws-samples/sample-pacer#76 ·
Los mantenedores suelen responder en 1 día
-
mime-type element is mandatoryAbierto
Dificultad 2/5 1-3 horas Aptitud para principiantes 68/100
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 68/100
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100
Los mantenedores suelen responder en 1 día
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 86/100
axodotdev/cargo-dist#2523 ·
Los mantenedores suelen responder en 2 días