apache / apache/iceberg

CreateSnapshotEvent / CommitReport are sent prematurely when using a Transaction

Open
#7,278 1 comment 0 reactions 0 assignees View on GitHub
not-stale
Dominant language
Java
Stars
9.2k
Forks
3.5k
Avg merge
2d 11h
Merged PRs (30d)
132

Description

### Apache Iceberg version

1.2.0 (latest release)

### Query engine

None

### Please describe the bug 🐞

Currently, a `CreateSnapshotEvent` or a `CommitReport` are both sent in [SnapshotProducer#notifyListeners()](https://github.com/apache/iceberg/blob/ba2a21506407df117bf55e6725c2ec4f44304898/core/src/main/java/org/apache/iceberg/SnapshotProducer.java#L432-L456) right after commit was successful.

However, when using a TX, that commit is only staged and only committed when the TX itself commits. This results in premature sending of `CreateSnapshotEvent` / `CommitReport` for a table that potentially won't be committed.

This can be reproduced by adding this to `TestCreateSnapshotEvent`
```
@Test
public void snapshotEventAvailableAfterTransactionCommit() {
Transaction transaction = table.newTransaction();
transaction.newAppend().appendFile(FILE_A).commit();

// the event should only be available after the TX was committed
Assertions.assertThat(currentEvent).isNull();

transaction.commitTransaction();
Assertions.assertThat(currentEvent).isNotNull();
}
```

and this to `TestCommitReporting`
```
@Test
public void commitReportAvailableAfterTransactionCommit() {
String tableName = "reporting-inside-tx";
Table table =
TestTables.create(
tableDir, tableName, SCHEMA, SPEC, SortOrder.unsorted(), formatVersion, reporter);

Transaction transaction = table.newTransaction();
transaction.newAppend().appendFile(FILE_A).commit();

assertThat(reporter.lastCommitReport()).isNull();

// commit report should only be available after the TX was committed
transaction.commitTransaction();

CommitReport report = reporter.lastCommitReport();
assertThat(report).isNotNull();
assertThat(report.operation()).isEqualTo("append");
assertThat(report.snapshotId()).isEqualTo(1L);
assertThat(report.sequenceNumber()).isEqualTo(1L);
assertThat(report.tableName()).isEqualTo(tableName);
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.