This is an automated email from the ASF dual-hosted git repository.
Gabriel39 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 62e3d81a42b [fix](iceberg) Refresh table before classifying expired
files (#69703)
62e3d81a42b is described below
commit 62e3d81a42b2b74ee2a5a577969df62edee06b78
Author: daidai <[email protected]>
AuthorDate: Wed Oct 28 18:53:36 2026 +0800
[fix](iceberg) Refresh table before classifying expired files (#69703)
### What problem does this PR solve?
Issue Number: N/A
Related PR: #69551
Problem Summary:
`expire_snapshots` built its delete-file kind map from a table handle
that could predate delete files committed by another writer. Iceberg
refreshes table metadata during expiration, so the callback could
receive an unmapped position or equality delete file and count it as a
data file.
Refresh the table before building the file-kind map so classification
and expiration operate on the same metadata generation. Add regression
coverage for position and equality delete files committed after the
table was loaded.
Before the fix, both cases returned `[1, 0, 0, 2, 3, 0]`. After the fix,
position deletes return `[0, 1, 0, 2, 3, 0]` and equality deletes return
`[0, 0, 1, 2, 3, 0]`.
### Release note
Fix `expire_snapshots` misreporting position and equality delete files
as data files after concurrent metadata commits.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason
Validated on latest `apache/master`: `IcebergExpireSnapshotsActionTest`,
15 tests passed. The full `fe-connector-iceberg` suite previously passed
1739 tests with 14 skipped.
- Behavior changed:
- [ ] No.
- [x] Yes. The reported position/equality delete counters are corrected.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
---
.../action/IcebergExpireSnapshotsAction.java | 4 ++
.../action/IcebergExpireSnapshotsActionTest.java | 47 ++++++++++++++++++++++
2 files changed, 51 insertions(+)
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/action/IcebergExpireSnapshotsAction.java
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/action/IcebergExpireSnapshotsAction.java
index d7ab40214d9..b6eafa09356 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/action/IcebergExpireSnapshotsAction.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/action/IcebergExpireSnapshotsAction.java
@@ -189,6 +189,10 @@ public class IcebergExpireSnapshotsAction extends
BaseIcebergAction {
ExecutorService deleteExecutor = null;
try {
+ // expireSnapshots() refreshes the table metadata before planning
expiration. Build the file-kind
+ // map from that same generation so delete files committed since
this table handle was loaded are
+ // not left unmapped and counted as data files by the delete
callback.
+ icebergTable.refresh();
Map<String, FileKind> fileKindByPath =
buildFileKindMap(icebergTable);
ExpireSnapshots expireSnapshots = icebergTable.expireSnapshots();
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/action/IcebergExpireSnapshotsActionTest.java
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/action/IcebergExpireSnapshotsActionTest.java
index 373d1ced7ae..b34f51efbd3 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/action/IcebergExpireSnapshotsActionTest.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/action/IcebergExpireSnapshotsActionTest.java
@@ -29,7 +29,9 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import org.apache.iceberg.DataFile;
import org.apache.iceberg.DataFiles;
+import org.apache.iceberg.DeleteFile;
import org.apache.iceberg.FileFormat;
+import org.apache.iceberg.FileMetadata;
import org.apache.iceberg.ManifestFile;
import org.apache.iceberg.PartitionSpec;
import org.apache.iceberg.Snapshot;
@@ -330,6 +332,51 @@ public class IcebergExpireSnapshotsActionTest {
"the equality-delete file must stay classified as
EQUALITY_DELETES");
}
+ @Test
+ public void countsPositionDeletesCommittedAfterTheTableWasLoaded() {
+ assertLateDeleteClassification(true);
+ }
+
+ @Test
+ public void countsEqualityDeletesCommittedAfterTheTableWasLoaded() {
+ assertLateDeleteClassification(false);
+ }
+
+ private static void assertLateDeleteClassification(boolean positionDelete)
{
+ InMemoryCatalog catalog = ActionTestTables.freshCatalog();
+ TableIdentifier id = ActionTestTables.id("late_delete");
+ ActionTestTables.createTable(catalog, "late_delete");
+ ActionTestTables.appendSnapshot(catalog, "late_delete",
"base.parquet", 1L);
+ Table loadedBeforeDelete = catalog.loadTable(id);
+
+ FileMetadata.Builder builder =
FileMetadata.deleteFileBuilder(PartitionSpec.unpartitioned());
+ if (positionDelete) {
+
builder.ofPositionDeletes().withReferencedDataFile("s3://b/db1/base.parquet");
+ } else {
+ builder.ofEqualityDeletes(1);
+ }
+ DeleteFile delete = builder.withPath("s3://b/db1/late-delete.parquet")
+ .withFormat(FileFormat.PARQUET)
+ .withFileSizeInBytes(64L)
+ .withRecordCount(1L)
+ .build();
+
+ Table writer = catalog.loadTable(id);
+ writer.newRowDelta().addDeletes(delete).commit();
+ writer.newRowDelta().removeDeletes(delete).commit();
+ ActionTestTables.appendSnapshot(catalog, "late_delete",
"current.parquet", 1L);
+
+ IcebergExpireSnapshotsAction action =
+ action(ImmutableMap.of("older_than", "4102444800000",
"retain_last", "1"));
+ action.validate();
+ List<String> row = action.execute(loadedBeforeDelete,
ActionTestTables.session("UTC")).getRows().get(0);
+
+ Assertions.assertEquals(1,
Iterables.size(catalog.loadTable(id).snapshots()),
+ "the delete-file history must actually expire");
+ Assertions.assertEquals(ImmutableList.of("0", positionDelete ? "1" :
"0", positionDelete ? "0" : "1",
+ "2", "3", "0"), row);
+ }
+
@Test
public void rejectsNegativeOlderThanTimestamp() {
DorisConnectorException e =
Assertions.assertThrows(DorisConnectorException.class,
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
