From bac8711353628cba6c76f694770530908ff5654d Mon Sep 17 00:00:00 2001 From: Gabor Kaszab Date: Fri, 28 Nov 2025 16:13:00 +0100 Subject: [PATCH] Core: Skip unnecessary metadata refresh after merge append With RESTTableOperations it's not required to perform a refresh after committing a merge append because the commit already refreshes table metadata. Initially an appendFiles().commit() required the following messages sent to REST catalog: 1) Load table at the beginning of SnapshotProducer.apply() 2) Update table to send the updates to the catalog 3) Load table again in MergingSnapshotProducer.updateEvent() The last step isn't needed when ops is RESTTableOperations. --- .../apache/iceberg/MergingSnapshotProducer.java | 7 ++++++- .../apache/iceberg/catalog/CatalogTests.java | 5 +++++ .../apache/iceberg/rest/TestRESTCatalog.java | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java b/core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java index 51d17fbdd0f2..761f94a830e1 100644 --- a/core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java +++ b/core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java @@ -968,7 +968,12 @@ public List apply(TableMetadata base, Snapshot snapshot) { @Override public Object updateEvent() { long snapshotId = snapshotId(); - Snapshot justSaved = ops().refresh().snapshot(snapshotId); + + Snapshot justSaved = ops().current().snapshot(snapshotId); + if (justSaved == null) { + justSaved = ops().refresh().snapshot(snapshotId); + } + long sequenceNumber = TableMetadata.INVALID_SEQUENCE_NUMBER; Map summary; if (justSaved == null) { diff --git a/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java b/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java index 833b2fb0b46f..90a513afeb8f 100644 --- a/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java +++ b/core/src/test/java/org/apache/iceberg/catalog/CatalogTests.java @@ -3271,6 +3271,11 @@ public void testCatalogWithCustomMetricsReporter() throws IOException { assertThat(CustomMetricsReporter.SCAN_COUNTER.get()).isEqualTo(1); // reset counter in case subclasses run this test multiple times CustomMetricsReporter.SCAN_COUNTER.set(0); + + // append file through MergeAppend and check and reset counter + table.newAppend().appendFile(FILE_A).commit(); + assertThat(CustomMetricsReporter.COMMIT_COUNTER.get()).isEqualTo(1); + CustomMetricsReporter.COMMIT_COUNTER.set(0); } public static class CustomMetricsReporter implements MetricsReporter { diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java index 6b981c493da0..0fc4a56aba5e 100644 --- a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java +++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java @@ -4326,6 +4326,23 @@ public void testTableCacheAgeDoesNotRefreshesAfterAccess() { .isEqualTo(HALF_OF_TABLE_EXPIRATION); } + @Test + public void testNumLoadTableCallsForMergeAppend() { + RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog)); + + RESTCatalog catalog = catalog(adapter); + + catalog.createNamespace(TABLE.namespace()); + + BaseTable table = (BaseTable) catalog.createTable(TABLE, SCHEMA); + + table.newAppend().appendFile(FILE_A).commit(); + + // loadTable is executed once + Mockito.verify(adapter) + .execute(reqMatcher(HTTPMethod.GET, RESOURCE_PATHS.table(TABLE)), any(), any(), any()); + } + private RESTCatalog catalog(RESTCatalogAdapter adapter) { RESTCatalog catalog = new RESTCatalog(SessionCatalog.SessionContext.createEmpty(), (config) -> adapter);