You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2026/08/27 20:09:16 UTC
(impala) branch master updated (f71f9284b -> b17a3b318)
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
from f71f9284b IMPALA-15308: Share Iceberg DML finalization
new 51b57a77c IMPALA-13314: Cache HadoopCatalog instances and use holder pattern for singletons
new b17a3b318 IMPALA-15273: Allow EC in plan output
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../org/apache/impala/catalog/IcebergTable.java | 21 ------------
.../impala/catalog/iceberg/IcebergCatalogUtil.java | 13 ++++---
.../catalog/iceberg/IcebergHadoopCatalog.java | 40 ++++++++++++++++------
.../catalog/iceberg/IcebergHadoopTables.java | 13 ++++---
.../impala/catalog/iceberg/IcebergHiveCatalog.java | 11 +++---
.../java/org/apache/impala/util/IcebergUtil.java | 26 +++++---------
.../queries/QueryTest/cte-distributed.test | 4 +--
.../functional-query/queries/QueryTest/cte.test | 4 +--
8 files changed, 59 insertions(+), 73 deletions(-)
(impala) 02/02: IMPALA-15273: Allow EC in plan output
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit b17a3b318d8b979299c540431c0b541c392813fa
Author: Michael Smith <mi...@cloudera.com>
AuthorDate: Tue Aug 25 16:08:49 2026 -0700
IMPALA-15273: Allow EC in plan output
Erasure-coded storage adds a new line to the plan output
erasure coded: files=1 size=2.15KB
which breaks with VERIFY_IS_EQUAL. Use VERIFY_IS_SUBSET to ignore it.
Testing: passed with ERASURE_CODING enabled.
Change-Id: Iafc193a9453de8dd102a5745021337d415c7b345
Reviewed-on: http://gerrit.cloudera.org:8080/24756
Reviewed-by: Michael Smith <mi...@cloudera.com>
Tested-by: Michael Smith <mi...@cloudera.com>
---
.../workloads/functional-query/queries/QueryTest/cte-distributed.test | 4 ++--
testdata/workloads/functional-query/queries/QueryTest/cte.test | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/testdata/workloads/functional-query/queries/QueryTest/cte-distributed.test b/testdata/workloads/functional-query/queries/QueryTest/cte-distributed.test
index 00447cea2..45e7cbd32 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/cte-distributed.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/cte-distributed.test
@@ -2,7 +2,7 @@
---- QUERY
EXPLAIN WITH v1 AS (SELECT n_name, n_nationkey FROM tpch.nation WHERE n_regionkey = 0)
SELECT v1.n_name, v1_next.n_name FROM v1, v1 v1_next WHERE v1.n_nationkey + 1 = v1_next.n_nationkey;
----- RESULTS: VERIFY_IS_EQUAL
+---- RESULTS: VERIFY_IS_SUBSET
'Max Per-Host Resource Reservation: Memory=20.95MB Threads=5'
'Per-Host Resource Estimates: Memory=53MB'
''
@@ -77,7 +77,7 @@ SET EXPLAIN_LEVEL=EXTENDED;
SET MT_DOP=2;
EXPLAIN SELECT name FROM emps e WHERE salary > 50000
UNION SELECT name FROM emps e WHERE salary > 50000;
----- RESULTS: VERIFY_IS_EQUAL
+---- RESULTS: VERIFY_IS_SUBSET
'Max Per-Host Resource Reservation: Memory=17.95MB Threads=4'
'Per-Host Resource Estimates: Memory=50MB'
''
diff --git a/testdata/workloads/functional-query/queries/QueryTest/cte.test b/testdata/workloads/functional-query/queries/QueryTest/cte.test
index 93ee8170a..dc42189ad 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/cte.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/cte.test
@@ -2,7 +2,7 @@
---- QUERY
EXPLAIN WITH v1 AS (SELECT n_name, n_nationkey FROM tpch.nation WHERE n_regionkey = 0)
SELECT v1.n_name, v1_next.n_name FROM v1, v1 v1_next WHERE v1.n_nationkey + 1 = v1_next.n_nationkey;
----- RESULTS: VERIFY_IS_EQUAL
+---- RESULTS: VERIFY_IS_SUBSET
'Max Per-Host Resource Reservation: Memory=20.95MB Threads=2'
'Per-Host Resource Estimates: Memory=53MB'
''
@@ -53,7 +53,7 @@ INSERT INTO emps VALUES
---- QUERY
EXPLAIN SELECT name FROM emps e WHERE salary > 50000
UNION SELECT name FROM emps e WHERE salary > 50000;
----- RESULTS: VERIFY_IS_EQUAL
+---- RESULTS: VERIFY_IS_SUBSET
'Max Per-Host Resource Reservation: Memory=52.01MB Threads=2'
'Per-Host Resource Estimates: Memory=178MB'
'WARNING: The following tables are missing relevant table and/or column statistics.'
(impala) 01/02: IMPALA-13314: Cache HadoopCatalog instances and use holder pattern for singletons
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 51b57a77c9f60a70c7b194386d3e1b406e78a643
Author: Nandor Kollar <ko...@gmail.com>
AuthorDate: Wed Aug 19 11:09:49 2026 +0200
IMPALA-13314: Cache HadoopCatalog instances and use holder pattern for singletons
Previously, a new HadoopCatalog (and its FileIO/filesystem resources)
was created on every table operation via getIcebergCatalog(). This
caused excessive resource consumption when many Iceberg tables shared
the same catalog location.
Changes:
- Introduce a Guava Cache with soft values in IcebergHadoopCatalog
keyed by catalog location string. Tables sharing the same
iceberg.catalog_location now reuse a single HadoopCatalog instance.
Entries are evicted under memory pressure via soft references.
- Replace synchronized lazy initialization with the
initialization-on-demand holder idiom for IcebergHiveCatalog,
IcebergHadoopTables, and IcebergCatalogUtil. The JVM guarantees
thread-safe lazy initialization without synchronization overhead on
every access.
- Convert getIcebergCatalog() to a switch expression, removing the
throws ImpalaRuntimeException declaration. Simplify loadTable()
by dropping the now-unnecessary try-catch.
- Remove unused getLastComputeStatsTimeMs() and
collectFieldIdsWithNdvStats() from IcebergTable.
- Fix logger in IcebergHadoopCatalog (was incorrectly using
IcebergHadoopTables.class).
- Make hadoopTables field final in IcebergHadoopTables.
Testing: Ran existing Iceberg catalog tests.
Assisted-by: Claude Opus 4.6 (Claude Code)
Change-Id: Ibb3a6c8e4f1d2a9e7c5b0f8d3e6a4c2b1d0e9f7a
Reviewed-on: http://gerrit.cloudera.org:8080/24712
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../org/apache/impala/catalog/IcebergTable.java | 21 ------------
.../impala/catalog/iceberg/IcebergCatalogUtil.java | 13 ++++---
.../catalog/iceberg/IcebergHadoopCatalog.java | 40 ++++++++++++++++------
.../catalog/iceberg/IcebergHadoopTables.java | 13 ++++---
.../impala/catalog/iceberg/IcebergHiveCatalog.java | 11 +++---
.../java/org/apache/impala/util/IcebergUtil.java | 26 +++++---------
6 files changed, 55 insertions(+), 69 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java b/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
index dbfc46c10..1336d506c 100644
--- a/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
@@ -27,7 +27,6 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -712,26 +711,6 @@ public class IcebergTable extends Table implements FeIcebergTable {
return Boolean.parseBoolean(val);
}
- private long getLastComputeStatsTimeMs() {
- String val = msTable_.getParameters().get(Table.TBL_PROP_LAST_COMPUTE_STATS_TIME);
- try {
- return Long.parseLong(val) * 1000;
- } catch (Exception e) {
- return -1;
- }
- }
-
- private Set<Integer> collectFieldIdsWithNdvStats() {
- Set<Integer> res = new HashSet<>();
- for (Column col : colsByPos_) {
- if (col.getStats().hasNumDistinctValues()) {
- IcebergColumn iCol = (IcebergColumn) col;
- res.add(iCol.getFieldId());
- }
- }
- return res;
- }
-
/**
* @throws TableLoadingException when it is unsafe to load the table.
*/
diff --git a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalogUtil.java b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalogUtil.java
index 6204c882e..0f6cea88a 100644
--- a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalogUtil.java
+++ b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCatalogUtil.java
@@ -47,19 +47,18 @@ import org.apache.impala.util.IcebergUtil;
* Supports custom catalog configurations via iceberg.catalog.<name>.* properties.
*/
public class IcebergCatalogUtil implements IcebergCatalog {
- private static IcebergCatalogUtil instance_;
-
public static final String CATALOGS_NAME_PROPERTY = "name";
public static final String CATALOGS_LOCATION_PROPERTY = "location";
public static final String CATALOG_CONFIG_PREFIX = "iceberg.catalog.";
// Based on InputFormatConfig.TABLE_IDENTIFIER
public static final String TABLE_IDENTIFIER = "iceberg.mr.table.identifier";
- public synchronized static IcebergCatalogUtil getInstance() {
- if (instance_ == null) {
- instance_ = new IcebergCatalogUtil();
- }
- return instance_;
+ private static final class Holder {
+ static final IcebergCatalogUtil INSTANCE = new IcebergCatalogUtil();
+ }
+
+ public static IcebergCatalogUtil getInstance() {
+ return Holder.INSTANCE;
}
private final Configuration configuration_;
diff --git a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
index 409aa78f1..f081bb692 100644
--- a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopCatalog.java
@@ -20,6 +20,7 @@ package org.apache.impala.catalog.iceberg;
import java.io.UncheckedIOException;
import java.lang.NullPointerException;
import java.util.Map;
+import java.util.concurrent.ExecutionException;
import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.PartitionSpec;
@@ -38,22 +39,39 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import com.google.common.base.Preconditions;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
/**
* Implementation of IcebergCatalog for tables stored in HadoopCatalog.
+ * Instances are cached by catalog location to avoid creating a new HadoopCatalog
+ * (and its associated FileIO) for each table operation.
*/
public class IcebergHadoopCatalog implements IcebergCatalog {
- private final static Logger LOG = LoggerFactory.getLogger(IcebergHadoopTables.class);
+ private final static Logger LOG = LoggerFactory.getLogger(IcebergHadoopCatalog.class);
- private HadoopCatalog hadoopCatalog;
+ private static final Cache<String, IcebergHadoopCatalog> catalogCache_ =
+ CacheBuilder.newBuilder().softValues().build();
- public IcebergHadoopCatalog(String catalogLocation) {
+ public static IcebergHadoopCatalog getInstance(String catalogLocation) {
+ try {
+ return catalogCache_.get(catalogLocation,
+ () -> new IcebergHadoopCatalog(catalogLocation));
+ } catch (ExecutionException e) {
+ throw new RuntimeException(
+ "Failed to create HadoopCatalog for location: " + catalogLocation, e);
+ }
+ }
+
+ private final HadoopCatalog hadoopCatalog_;
+
+ private IcebergHadoopCatalog(String catalogLocation) {
setContextClassLoader();
- hadoopCatalog = new HadoopCatalog();
+ hadoopCatalog_ = new HadoopCatalog();
Map<String, String> props = IcebergUtil.composeCatalogProperties();
props.put(CatalogProperties.WAREHOUSE_LOCATION, catalogLocation);
- hadoopCatalog.setConf(FileSystemUtil.getConfiguration());
- hadoopCatalog.initialize("", props);
+ hadoopCatalog_.setConf(FileSystemUtil.getConfiguration());
+ hadoopCatalog_.initialize("", props);
}
@Override
@@ -64,7 +82,7 @@ public class IcebergHadoopCatalog implements IcebergCatalog {
String location,
Map<String, String> properties) {
// We pass null as 'location' to let Iceberg decide the table location.
- return hadoopCatalog.createTable(identifier, schema, spec, /*location=*/null,
+ return hadoopCatalog_.createTable(identifier, schema, spec, /*location=*/null,
properties);
}
@@ -85,7 +103,7 @@ public class IcebergHadoopCatalog implements IcebergCatalog {
int attempt = 0;
while (attempt < MAX_ATTEMPTS) {
try {
- return hadoopCatalog.loadTable(tableId);
+ return hadoopCatalog_.loadTable(tableId);
} catch (NoSuchTableException e) {
throw new IcebergTableLoadingException(e.getMessage());
} catch (NullPointerException | UncheckedIOException e) {
@@ -113,19 +131,19 @@ public class IcebergHadoopCatalog implements IcebergCatalog {
Preconditions.checkState(
feTable.getIcebergCatalog() == TIcebergCatalog.HADOOP_CATALOG);
TableIdentifier tableId = IcebergUtil.getIcebergTableIdentifier(feTable);
- return hadoopCatalog.dropTable(tableId, purge);
+ return hadoopCatalog_.dropTable(tableId, purge);
}
@Override
public boolean dropTable(String dbName, String tblName, boolean purge) {
- return hadoopCatalog.dropTable(TableIdentifier.of(dbName, tblName), purge);
+ return hadoopCatalog_.dropTable(TableIdentifier.of(dbName, tblName), purge);
}
@Override
public void renameTable(FeIcebergTable feTable, TableIdentifier newTableId) {
TableIdentifier oldTableId = IcebergUtil.getIcebergTableIdentifier(feTable);
try {
- hadoopCatalog.renameTable(oldTableId, newTableId);
+ hadoopCatalog_.renameTable(oldTableId, newTableId);
} catch (UnsupportedOperationException e) {
throw new UnsupportedOperationException(
"Cannot rename Iceberg tables that use 'hadoop.catalog' as catalog.");
diff --git a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
index 09934c0e6..90ebd6324 100644
--- a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
+++ b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHadoopTables.java
@@ -43,16 +43,15 @@ import com.google.common.base.Preconditions;
public class IcebergHadoopTables implements IcebergCatalog {
private final static Logger LOG = LoggerFactory.getLogger(IcebergHadoopTables.class);
- private static IcebergHadoopTables instance_;
+ private static final class Holder {
+ static final IcebergHadoopTables INSTANCE = new IcebergHadoopTables();
+ }
- public synchronized static IcebergHadoopTables getInstance() {
- if (instance_ == null) {
- instance_ = new IcebergHadoopTables();
- }
- return instance_;
+ public static IcebergHadoopTables getInstance() {
+ return Holder.INSTANCE;
}
- private HadoopTables hadoopTables;
+ private final HadoopTables hadoopTables;
private IcebergHadoopTables() {
hadoopTables = new HadoopTables(FileSystemUtil.getConfiguration());
diff --git a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHiveCatalog.java b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHiveCatalog.java
index 40e01b028..e5d24bba5 100644
--- a/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHiveCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergHiveCatalog.java
@@ -41,13 +41,12 @@ import com.google.common.base.Preconditions;
* Implementation of IcebergCatalog for tables stored in HiveCatalog.
*/
public class IcebergHiveCatalog implements IcebergCatalog {
- private static IcebergHiveCatalog instance_;
+ private static final class Holder {
+ static final IcebergHiveCatalog INSTANCE = new IcebergHiveCatalog();
+ }
- public synchronized static IcebergHiveCatalog getInstance() {
- if (instance_ == null) {
- instance_ = new IcebergHiveCatalog();
- }
- return instance_;
+ public static IcebergHiveCatalog getInstance() {
+ return Holder.INSTANCE;
}
private final HiveCatalog hiveCatalog_;
diff --git a/fe/src/main/java/org/apache/impala/util/IcebergUtil.java b/fe/src/main/java/org/apache/impala/util/IcebergUtil.java
index 2bf99add3..627cf90b6 100644
--- a/fe/src/main/java/org/apache/impala/util/IcebergUtil.java
+++ b/fe/src/main/java/org/apache/impala/util/IcebergUtil.java
@@ -177,15 +177,13 @@ public class IcebergUtil {
/**
* Returns the corresponding catalog implementation.
*/
- public static IcebergCatalog getIcebergCatalog(TIcebergCatalog catalog, String location)
- throws ImpalaRuntimeException {
- switch (catalog) {
- case HADOOP_TABLES: return IcebergHadoopTables.getInstance();
- case HIVE_CATALOG: return IcebergHiveCatalog.getInstance();
- case HADOOP_CATALOG: return new IcebergHadoopCatalog(location);
- case CATALOGS: return IcebergCatalogUtil.getInstance();
- default: throw new ImpalaRuntimeException("Unexpected catalog type: " + catalog);
- }
+ public static IcebergCatalog getIcebergCatalog(TIcebergCatalog catalog, String location) {
+ return switch (catalog) {
+ case HADOOP_TABLES -> IcebergHadoopTables.getInstance();
+ case HIVE_CATALOG -> IcebergHiveCatalog.getInstance();
+ case HADOOP_CATALOG -> IcebergHadoopCatalog.getInstance(location);
+ case CATALOGS -> IcebergCatalogUtil.getInstance();
+ };
}
/**
@@ -203,14 +201,8 @@ public class IcebergUtil {
public static Table loadTable(TIcebergCatalog catalog, TableIdentifier tableId,
String location, Map<String, String> tableProps)
throws IcebergTableLoadingException {
- try {
- IcebergCatalog cat = getIcebergCatalog(catalog, location);
- return cat.loadTable(tableId, location, tableProps);
- } catch (ImpalaRuntimeException e) {
- throw new IcebergTableLoadingException(String.format(
- "Failed to load Iceberg table: %s at location: %s",
- tableId, location), e);
- }
+ IcebergCatalog cat = getIcebergCatalog(catalog, location);
+ return cat.loadTable(tableId, location, tableProps);
}
/**