This is an automated email from the ASF dual-hosted git repository. boroknagyz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit aaf6fdc645b2f67e67f009c8e6dcf3ce25b22c98 Author: Zoltan Borok-Nagy <[email protected]> AuthorDate: Fri Sep 23 16:35:47 2022 +0200 IMPALA-11508: Deflake test_expire_snapshots Before this patch test_expire_snapshots failed frequently. The patch is only a few lines of code, but there are some subtleties here: IcebergCatalogOpExecutor.alterTableExecute() didn't use Iceberg transactions to carry out the operation. This means that expireSnapshots() resulted in an ALTER TABLE operation on its own which we also got during event processing. Because this ALTER TABLE event didn't had the catalog version set we didn't recognized it as a self-event. This caused unnecessary table reloads during the tests which manifested in InconsistentMetadataFetchException: "... table ... changed version between accesses" errors. With this patch IcebergCatalogOpExecutor.alterTableExecute() takes an Iceberg transaction object and invokes expireSnapshots() in the context of this Iceberg transaction. This Iceberg transaction also sets table properties "impala.events.catalogServiceId" and "impala.events.catalogVersion". And because everything happens in a single Iceberg transaction we only create a single ALTER TABLE which we can recognize during event processing (based on the table properties), avoiding unnecessary table reloads. Testing: * executed test_expire_snapshots in a loop Change-Id: I6d82c8b52466a24af096fe5fe4dbd034a1ee6a15 Reviewed-on: http://gerrit.cloudera.org:8080/19036 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java | 2 +- .../main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java index 4771375f4..6a9e833e1 100644 --- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java +++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java @@ -1326,7 +1326,7 @@ public class CatalogOpExecutor { break; case EXECUTE: Preconditions.checkState(params.isSetSet_execute_params()); - String summary = IcebergCatalogOpExecutor.alterTableExecute(tbl, + String summary = IcebergCatalogOpExecutor.alterTableExecute(iceTxn, params.getSet_execute_params()); addSummary(response, summary); break; diff --git a/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java index 2a482e2bb..75aa6e9a0 100644 --- a/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java +++ b/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java @@ -177,9 +177,9 @@ public class IcebergCatalogOpExecutor { tableOp.commit(metadata, newMetadata); } - public static String alterTableExecute(FeIcebergTable tbl, + public static String alterTableExecute(Transaction txn, TAlterTableExecuteParams params) { - ExpireSnapshots expireApi = tbl.getIcebergApiTable().expireSnapshots(); + ExpireSnapshots expireApi = txn.expireSnapshots(); expireApi.expireOlderThan(params.older_than_millis); expireApi.commit(); return "Snapshots have been expired.";
