diff --git a/standalone-metastore/metastore-rest-catalog/pom.xml b/standalone-metastore/metastore-rest-catalog/pom.xml index edeb7de1d730..f5f7ef7a79f9 100644 --- a/standalone-metastore/metastore-rest-catalog/pom.xml +++ b/standalone-metastore/metastore-rest-catalog/pom.xml @@ -314,6 +314,17 @@ org.apache.maven.plugins maven-surefire-plugin + + + false + org.apache.maven.plugins diff --git a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/hive/MetadataLocator.java b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/hive/MetadataLocator.java new file mode 100644 index 000000000000..f6acc758b894 --- /dev/null +++ b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/hive/MetadataLocator.java @@ -0,0 +1,112 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iceberg.hive; + +import java.util.Collections; +import java.util.List; + +import org.apache.hadoop.hive.metastore.IMetaStoreClient; +import org.apache.hadoop.hive.metastore.api.GetProjectionsSpec; +import org.apache.hadoop.hive.metastore.api.NoSuchObjectException; +import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.client.builder.GetTableProjectionsSpecBuilder; +import org.apache.iceberg.BaseMetastoreTableOperations; +import org.apache.iceberg.ClientPool; +import org.apache.iceberg.MetadataTableType; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.thrift.TException; + +/** + * Fetches the location of a given metadata table. + *

Since the location mutates with each transaction, this allows determining if a cached version of the + * table is the latest known in the HMS database.

+ */ +public class MetadataLocator { + private static final org.slf4j.Logger LOGGER = org.slf4j.LoggerFactory.getLogger(MetadataLocator.class); + private static final GetProjectionsSpec PARAM_SPEC = + new GetTableProjectionsSpecBuilder() + .includeParameters() // only fetches table.parameters + .build(); + private final HiveCatalog catalog; + + public MetadataLocator(HiveCatalog catalog) { + this.catalog = catalog; + } + + public HiveCatalog getCatalog() { + return catalog; + } + + /** + * Returns the current metadata-file location of the table identified by the given identifier. The + * identifier may be either a base table (e.g. {@code db.table}) or one of its metadata tables + * (e.g. {@code db.table.snapshots}), which is resolved to its base table before the lookup. + *

This uses the Thrift API to fetch the table parameters, which is more efficient than fetching the entire table object.

+ * @param identifier the base-table or metadata-table identifier to fetch the location for + * @return the current metadata-file location, or null if the table (or its database/catalog) does + * not exist, or the identifier is neither a valid table nor a valid metadata-table identifier + * @throws RuntimeException if the HMS lookup fails for any reason other than the object not existing + */ + public String getLocation(TableIdentifier identifier) { + final ClientPool clients = catalog.clientPool(); + final String catName = catalog.name(); + final TableIdentifier baseTableIdentifier; + if (!catalog.isValidIdentifier(identifier)) { + if (!isValidMetadataIdentifier(identifier)) { + return null; + } else { + baseTableIdentifier = TableIdentifier.of(identifier.namespace().levels()); + } + } else { + baseTableIdentifier = identifier; + } + String database = baseTableIdentifier.namespace().level(0); + String tableName = baseTableIdentifier.name(); + try { + List tables = + clients.run(client -> client.getTables(catName, database, Collections.singletonList(tableName), PARAM_SPEC)); + if (tables != null && !tables.isEmpty()) { + Table table = tables.getFirst(); + if (table != null) { + HiveOperationsBase.validateIcebergViewNotLoadedAsIcebergTable(table, baseTableIdentifier.toString()); + return table.getParameters().get(BaseMetastoreTableOperations.METADATA_LOCATION_PROP); + } + } + return null; + } catch (NoSuchObjectException e) { + // NoSuchObjectException is a TException subclass HMS raises for an unknown database or catalog. + // Like an empty getTables result, it means the object does not exist, so we return null and let + // callers treat null uniformly as not-found (matching the missing-table case above). + LOGGER.debug("Table {} not found: {}", baseTableIdentifier, e.getMessage()); + return null; + } catch (TException e) { + LOGGER.warn("Table {} parameters fetch failed: {}", baseTableIdentifier, e.getMessage()); + throw new RuntimeException("Failed to fetch table parameters for " + baseTableIdentifier, e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Interrupted while fetching table parameters for " + baseTableIdentifier, e); + } + } + + private boolean isValidMetadataIdentifier(TableIdentifier identifier) { + return MetadataTableType.from(identifier.name()) != null + && catalog.isValidIdentifier(TableIdentifier.of(identifier.namespace().levels())); + } +} diff --git a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCachingCatalog.java b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCachingCatalog.java index a78bd6cfbf5d..7fa509c1ce39 100644 --- a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCachingCatalog.java +++ b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCachingCatalog.java @@ -19,12 +19,33 @@ package org.apache.iceberg.rest; -import com.github.benmanes.caffeine.cache.Ticker; +import java.io.Closeable; +import java.lang.management.ManagementFactory; +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; -import org.apache.iceberg.CachingCatalog; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.LongAdder; + +import javax.management.JMException; +import javax.management.MBeanServer; +import javax.management.ObjectName; + +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.Ticker; +import org.apache.hadoop.conf.Configuration; +import org.apache.iceberg.BaseMetadataTable; +import org.apache.iceberg.HasTableOperations; +import org.apache.iceberg.MetadataTableType; +import org.apache.iceberg.MetadataTableUtils; import org.apache.iceberg.Schema; +import org.apache.iceberg.Table; +import org.apache.iceberg.TableOperations; import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.catalog.Namespace; import org.apache.iceberg.catalog.SupportsNamespaces; @@ -32,59 +53,545 @@ import org.apache.iceberg.catalog.ViewCatalog; import org.apache.iceberg.exceptions.NamespaceNotEmptyException; import org.apache.iceberg.exceptions.NoSuchNamespaceException; +import org.apache.iceberg.exceptions.NoSuchTableException; import org.apache.iceberg.hive.HiveCatalog; +import org.apache.iceberg.hive.MetadataLocator; import org.apache.iceberg.view.View; import org.apache.iceberg.view.ViewBuilder; - +import org.jetbrains.annotations.TestOnly; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** - * Class that wraps an Iceberg Catalog to cache tables. + * Caching wrapper around a {@link HiveCatalog} that adds two-level table caching. + * + *

Table caching (L2 + L1)

+ *

L2 — Caffeine cache. The primary table store. Each {@link Table} object is keyed by + * its {@link TableIdentifier} and expires after the configured inactivity period + * ({@code ICEBERG_CATALOG_CACHE_EXPIRY}, in milliseconds). On a cache miss, the table is loaded + * from the underlying {@link HiveCatalog} and its current metadata location is recorded. + * Subsequent hits skip the HMS round-trip entirely.

+ * + *

L1 — LinkedHashMap recency guard. A small bounded, access-ordered (LRU) map (default + * 32 entries, 3 s TTL; configurable via {@code hms.caching.catalog.l1.cache.size} and + * {@code hms.caching.catalog.l1.cache.ttl}) that tracks when each L2-cached table was last + * confirmed fresh; when it overflows, the least-recently-used entry is evicted. While the L1 entry is live, {@code loadTable} skips the metadata-location + * staleness check against HMS. Once the L1 entry expires, the next call re-validates the stored + * metadata location; if it has changed, the L2 entry is evicted ({@code onCacheInvalidate}) and + * a fresh load is performed. The L1 layer trades a small risk of serving a stale snapshot for a + * large reduction in HMS round-trips under repeated access to the same table.

+ * + *

Both cache levels are invalidated together by {@link #invalidateTable(TableIdentifier)}, + * which also evicts all derived {@link org.apache.iceberg.MetadataTableType metadata-table} + * entries that share the base identifier.

+ * + *

Observability

+ *

This class implements {@link HMSCachingCatalogMXBean} and registers itself with the platform + * MBean server under the name {@code org.apache.iceberg.rest:type=HMSCachingCatalog,name=<catalogName>} + * so that cache hit/miss counts and invalidation counts can be monitored via JMX. The + * {@code catalogName} is {@link org.apache.iceberg.catalog.Catalog#name()} of the wrapped catalog + * (the metastore's configured default catalog name), sanitized for use in an {@link ObjectName}.

*/ -public class HMSCachingCatalog extends CachingCatalog implements SupportsNamespaces, ViewCatalog { +public final class HMSCachingCatalog + implements Catalog, SupportsNamespaces, ViewCatalog, HMSCachingCatalogMXBean, Closeable { + private static final Logger LOG = LoggerFactory.getLogger(HMSCachingCatalog.class); + + /** + * Returns the underlying {@link HiveCatalog} that this caching catalog wraps. + * This is intended for testing purposes only; production code should not rely on the underlying catalog. + * @return the underlying HiveCatalog + */ + @TestOnly + public HiveCatalog getCatalog() { + return hiveCatalog; + } + + // The underlying HiveCatalog that this caching catalog wraps. private final HiveCatalog hiveCatalog; - - public HMSCachingCatalog(HiveCatalog catalog, long expiration) { - super(catalog, true, expiration, Ticker.systemTicker()); + // Authorizes reads served from the cache. A cache hit never reaches Hive Metastore, so its read + // authorization cannot be deferred to HMS and is enforced here instead. May be null (no + // authorization), in which case cache hits are served without a check. + private final IcebergAuthorizer authorizer; + // A helper that locates the metadata location for a given base table identifier. + private final MetadataLocator metadataLocator; + // An L2 table cache (Caffeine). + private final Cache tableCache; + // An L1 small latency cache. + // This is used to cache the last cached time for each table identifier, + // so that we can skip location check for repeated access to the same table within a short period of time, + // which can significantly reduce the latency for repeated access to the same table. + private final Map l1Cache; + // The TTL for L1 cache (3s). + private final int l1Ttl; + // The L1 cache size. + private final int l1CacheSize; + // Metrics counters. + private final LongAdder cacheHitCount = new LongAdder(); + private final LongAdder cacheMissCount = new LongAdder(); + private final LongAdder cacheLoadCount = new LongAdder(); + private final LongAdder cacheInvalidateCount = new LongAdder(); + private final LongAdder cacheMetaLoadCount = new LongAdder(); + // L1 cache metrics: counted only when the L2 (Caffeine) cache already has the entry. + private final LongAdder l1CacheHitCount = new LongAdder(); + private final LongAdder l1CacheMissCount = new LongAdder(); + // JMX ObjectName under which this instance is registered (may be null if registration failed). + private ObjectName jmxObjectName; + + /** + * Creates a new caching catalog that wraps the given HiveCatalog, without cache-hit + * authorization. + * @param catalog the underlying HiveCatalog + * @param expirationMs the expiration time for the L2 cache, in milliseconds + */ + public HMSCachingCatalog(HiveCatalog catalog, long expirationMs) { + this(catalog, expirationMs, null); + } + + /** + * Creates a new caching catalog that wraps the given HiveCatalog. + * @param catalog the underlying HiveCatalog + * @param expirationMs the expiration time for the L2 cache, in milliseconds + * @param authorizer authorizes reads served from the cache; may be null for no authorization. + * A cache hit does not reach Hive Metastore, so read authorization for it is + * enforced here rather than deferred to HMS. Cache misses reload through the + * underlying {@link HiveCatalog} and are authorized by HMS as usual. + */ + public HMSCachingCatalog(HiveCatalog catalog, long expirationMs, IcebergAuthorizer authorizer) { this.hiveCatalog = catalog; + this.authorizer = authorizer; + this.metadataLocator = new MetadataLocator(catalog); + this.tableCache = Caffeine.newBuilder() + .expireAfterAccess(expirationMs, TimeUnit.MILLISECONDS) + .ticker(Ticker.systemTicker()) + .build(); + Configuration conf = catalog.getConf(); + int l1size = conf.getInt("hms.caching.catalog.l1.cache.size", 32); + int l1ttl = conf.getInt("hms.caching.catalog.l1.cache.ttl", 3_000); + if (l1size > 0 && l1ttl > 0) { + // Access-ordered (LRU) so that re-confirming a hot table via l1MarkFresh (a put on an + // existing key) moves it to the tail; the eldest evicted by removeEldestEntry is then the + // least-recently-used entry rather than the least-recently-inserted one. + l1Cache = Collections.synchronizedMap(new LinkedHashMap(l1size, 0.75f, true) { + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return size() > l1CacheSize; + } + }); + l1Ttl = l1ttl; + l1CacheSize = l1size; + } else { + l1Cache = Collections.emptyMap(); + l1Ttl = 0; + l1CacheSize = 0; + } + // Register this instance as a JMX MBean for monitoring. The catalog was initialized with the + // metastore's CATALOG_DEFAULT name (see HMSCatalogFactory), so catalog.name() already yields + // that value; using it directly keeps this class free of a Configuration/MetastoreConf + // dependency and reflects the actual identity of the wrapped catalog. + registerJmx(catalog.name()); } + /** + * Registers this instance as a JMX MBean. + * + * @param catalogName the catalog name, used to build the {@link ObjectName} + */ + private void registerJmx(String catalogName) { + try { + MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); + String sanitized = catalogName == null || catalogName.isEmpty() + ? "default" + : catalogName.replaceAll("[^a-zA-Z0-9.\\\\-]", "_"); + ObjectName name = new ObjectName("org.apache.iceberg.rest:type=HMSCachingCatalog,name=" + sanitized); + if (mbs.isRegistered(name)) { + mbs.unregisterMBean(name); + } + mbs.registerMBean(this, name); + this.jmxObjectName = name; + LOG.info("Registered JMX MBean: {}", name); + } catch (JMException e) { + LOG.error("Failed to register JMX MBean for HMSCachingCatalog", e); + } + } + + /** + * Callback when cache invalidates the entry for a given table identifier. + * + * @param tid the table identifier to invalidate + */ + private void onCacheInvalidate(TableIdentifier tid) { + cacheInvalidateCount.increment(); + if (LOG.isDebugEnabled()) { + LOG.debug("Cache invalidate {}: {}", tid, cacheInvalidateCount.sum()); + } + } + + /** + * Callback when cache loads a table for a given table identifier. + * + * @param tid the table identifier + */ + private void onCacheLoad(TableIdentifier tid) { + cacheLoadCount.increment(); + if (LOG.isDebugEnabled()) { + LOG.debug("Cache load {}: {}", tid, cacheLoadCount.sum()); + } + } + + /** + * Callback when cache hit for a given table identifier. + * + * @param tid the table identifier + */ + private void onCacheHit(TableIdentifier tid) { + cacheHitCount.increment(); + if (LOG.isDebugEnabled()) { + LOG.debug("Cache hit {} : {}", tid, cacheHitCount.sum()); + } + } + + /** + * Callback when cache miss occurs for a given table identifier. + * + * @param tid the table identifier + */ + private void onCacheMiss(TableIdentifier tid) { + cacheMissCount.increment(); + if (LOG.isDebugEnabled()) { + LOG.debug("Cache miss {}: {}", tid, cacheMissCount.sum()); + } + } + + /** + * Callback when cache loads a metadata table for a given table identifier. + * + * @param tid the table identifier + */ + private void onCacheMetaLoad(TableIdentifier tid) { + cacheMetaLoadCount.increment(); + if (LOG.isDebugEnabled()) { + LOG.debug("Cache meta-load {}: {}", tid, cacheMetaLoadCount.sum()); + } + } + + /** + * Callback when an L1 cache hit occurs for a given table identifier. + * Only fired when the L2 cache also has the entry. + * + * @param tid the table identifier + */ + private void onL1CacheHit(TableIdentifier tid) { + l1CacheHitCount.increment(); + if (LOG.isDebugEnabled()) { + LOG.debug("L1 cache hit {}: {}", tid, l1CacheHitCount.sum()); + } + } + + /** + * Callback when an L1 cache miss occurs for a given table identifier. + * Only fired when the L2 cache has the entry but L1 is absent or expired. + * + * @param tid the table identifier + */ + private void onL1CacheMiss(TableIdentifier tid) { + l1CacheMissCount.increment(); + if (LOG.isDebugEnabled()) { + LOG.debug("L1 cache miss {}: {}", tid, l1CacheMissCount.sum()); + } + } + + // Getter methods for accessing metrics @Override - public Catalog.TableBuilder buildTable(TableIdentifier identifier, Schema schema) { - return hiveCatalog.buildTable(identifier, schema); + public long getCacheHitCount() { + return cacheHitCount.sum(); + } + + @Override + public long getCacheMissCount() { + return cacheMissCount.sum(); + } + + @Override + public long getCacheLoadCount() { + return cacheLoadCount.sum(); + } + + @Override + public long getCacheInvalidateCount() { + return cacheInvalidateCount.sum(); + } + + @Override + public long getCacheMetaLoadCount() { + return cacheMetaLoadCount.sum(); + } + + @Override + public double getCacheHitRate() { + long hits = cacheHitCount.sum(); + long total = hits + cacheMissCount.sum(); + return total == 0 ? 0.0 : (double) hits / total; + } + + @Override + public long getL1CacheHitCount() { + return l1CacheHitCount.sum(); + } + + @Override + public long getL1CacheMissCount() { + return l1CacheMissCount.sum(); + } + + @Override + public double getL1CacheHitRate() { + long hits = l1CacheHitCount.sum(); + long total = hits + l1CacheMissCount.sum(); + return total == 0 ? 0.0 : (double) hits / total; } @Override - public void createNamespace(Namespace nmspc, Map map) { - hiveCatalog.createNamespace(nmspc, map); + public void resetCacheStats() { + cacheHitCount.reset(); + cacheMissCount.reset(); + cacheLoadCount.reset(); + cacheInvalidateCount.reset(); + cacheMetaLoadCount.reset(); + l1CacheHitCount.reset(); + l1CacheMissCount.reset(); + LOG.debug("Cache stats reset"); } @Override - public List listNamespaces(Namespace nmspc) throws NoSuchNamespaceException { - return hiveCatalog.listNamespaces(nmspc); + public void close() { + unregisterJmx(); + } + + /** + * Unregisters this instance from the platform MBeanServer. + */ + private void unregisterJmx() { + if (jmxObjectName != null) { + try { + MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); + if (mbs.isRegistered(jmxObjectName)) { + mbs.unregisterMBean(jmxObjectName); + LOG.info("Unregistered JMX MBean: {}", jmxObjectName); + } + } catch (JMException e) { + LOG.warn("Failed to unregister JMX MBean: {}", jmxObjectName, e); + } finally { + jmxObjectName = null; + } + } } @Override - public Map loadNamespaceMetadata(Namespace nmspc) throws NoSuchNamespaceException { - return hiveCatalog.loadNamespaceMetadata(nmspc); + public String name() { + return hiveCatalog.name(); } @Override - public boolean dropNamespace(Namespace nmspc) throws NamespaceNotEmptyException { - List tables = listTables(nmspc); - for (TableIdentifier ident : tables) { + public List listTables(Namespace namespace) { + return hiveCatalog.listTables(namespace); + } + + @Override + public boolean dropTable(TableIdentifier identifier, boolean purge) { + boolean dropped = hiveCatalog.dropTable(identifier, purge); + invalidateTable(identifier); + return dropped; + } + + @Override + public void renameTable(TableIdentifier from, TableIdentifier to) { + hiveCatalog.renameTable(from, to); + invalidateTable(from); + } + + @Override + public Table registerTable(TableIdentifier identifier, String metadataFileLocation) { + Table registered = hiveCatalog.registerTable(identifier, metadataFileLocation); + invalidateTable(identifier); + return registered; + } + + @Override + public void invalidateTable(TableIdentifier ident) { + hiveCatalog.invalidateTable(ident); + tableCache.invalidate(ident); + tableCache.invalidateAll(metadataTableIdentifiers(ident)); + l1Invalidate(ident); + } + + /** + * Records {@code now} as the last time the given identifier was confirmed fresh in the L1 + * recency guard. No-op when L1 is disabled: in that case {@link #l1Cache} is an immutable empty + * map, so writing to it would throw {@link UnsupportedOperationException}. + */ + private void l1MarkFresh(TableIdentifier ident, long now) { + if (l1Ttl > 0) { + l1Cache.put(ident, now); + } + } + + /** Evicts the given identifier from the L1 recency guard. No-op when L1 is disabled. */ + private void l1Invalidate(TableIdentifier ident) { + if (l1Ttl > 0) { + l1Cache.remove(ident); + } + } + + /** + * Returns the identifiers of all metadata tables derived from the given base table identifier, + * in both upper-case and lower-case type-name forms so that eviction covers both variants. + */ + private List metadataTableIdentifiers(TableIdentifier identifier) { + MetadataTableType[] types = MetadataTableType.values(); + List result = new ArrayList<>(types.length * 2); + for (MetadataTableType type : types) { + result.add(TableIdentifier.parse(identifier + "." + type.name())); + result.add(TableIdentifier.parse(identifier + "." + type.name().toLowerCase(Locale.ROOT))); + } + return result; + } + + @Override + public void createNamespace(Namespace namespace, Map map) { + hiveCatalog.createNamespace(namespace, map); + } + + @Override + public List listNamespaces(Namespace namespace) throws NoSuchNamespaceException { + return hiveCatalog.listNamespaces(namespace); + } + + @Override + public void invalidateView(TableIdentifier identifier) { + hiveCatalog.invalidateView(identifier); + } + + /** + * Authorizes a read that is about to be served from the cache. A cache hit never reaches Hive + * Metastore, so its read authorization cannot be deferred to the HMS pre-event listener as a cache + * miss's can, and must be enforced here. No-op when no authorizer is configured. + * + * @param identifier the table (or metadata-table) identifier being read + * @throws org.apache.iceberg.exceptions.ForbiddenException if the current user may not read the table + */ + private void authorizeCachedRead(TableIdentifier identifier) { + if (authorizer != null) { + authorizer.authorizeLoadTable(hiveCatalog.name(), identifier); + } + } + + @Override + public Table loadTable(final TableIdentifier identifier) { + final Table cachedTable = tableCache.getIfPresent(identifier); + long now = System.currentTimeMillis(); + if (cachedTable != null) { + // Determine if L1 cache is valid based on the last cached time and the TTL. + // If the table is in L1 cache, we can skip the location check and return the cached table directly, + // which can significantly reduce the latency for repeated access to the same table. + Long lastCached = l1Cache.get(identifier); + if (lastCached != null) { + if (now - lastCached < l1Ttl) { + LOG.debug("Table {} is in L1 cache, returning cached table", identifier); + onL1CacheHit(identifier); + onCacheHit(identifier); + authorizeCachedRead(identifier); + return cachedTable; + } else { + l1Invalidate(identifier); + onL1CacheMiss(identifier); + } + } else { + onL1CacheMiss(identifier); + } + // If the table is no longer in L1 cache, we need to check the location. + final String location = metadataLocator.getLocation(identifier); + if (location == null) { + // A null location means the table no longer exists in HMS. The cached instance is stale and + // its metadata/manifests are highly likely to be deleted, so we must not serve it: evict the + // entry and signal not-found rather than returning a ghost table. + LOG.debug("Table {} no longer exists in HMS, evicting stale cache entry", identifier); + invalidateTable(identifier); + throw new NoSuchTableException("Table does not exist: %s", identifier); + } + String cachedLocation = + cachedTable instanceof HasTableOperations tableOps ? tableOps.operations().current().metadataFileLocation() : null; + if (location.equals(cachedLocation)) { + onCacheHit(identifier); + l1MarkFresh(identifier, now); + authorizeCachedRead(identifier); + return cachedTable; + } else { + LOG.debug("Invalidate table {}, cached {} != actual {}", identifier, cachedLocation, location); + // Invalidate the cached table if the location is different + invalidateTable(identifier); + onCacheInvalidate(identifier); + } + } else { + onCacheMiss(identifier); + } + final Table table = tableCache.get(identifier, this::loadTableWithoutCache); + if (table instanceof BaseMetadataTable) { + // Cache underlying table: there must be a table named by the namespace (?) + TableIdentifier originTableIdentifier = TableIdentifier.of(identifier.namespace().levels()); + Table originTable = tableCache.get(originTableIdentifier, this::loadTableWithoutCache); + // Share TableOperations instance of origin table for all metadata tables, so that metadata + // table instances are refreshed as well when origin table instance is refreshed. + if (originTable instanceof HasTableOperations tableOps) { + TableOperations ops = tableOps.operations(); + MetadataTableType type = MetadataTableType.from(identifier.name()); + // Defensive: MetadataTableType.from may return null for unknown names + if (type != null) { + Table metadataTable = + MetadataTableUtils.createMetadataTableInstance(ops, hiveCatalog.name(), originTableIdentifier, identifier, type); + tableCache.put(identifier, metadataTable); + l1MarkFresh(identifier, now); + onCacheMetaLoad(identifier); + LOG.debug("Loaded metadata table: {} for origin table: {}", identifier, originTableIdentifier); + // Return the metadata table instead of the original table + return metadataTable; + } + } + } + l1MarkFresh(identifier, now); + onCacheLoad(identifier); + return table; + } + + @Override + public boolean tableExists(TableIdentifier identifier) { + return metadataLocator.getLocation(identifier) != null; + } + + private Table loadTableWithoutCache(TableIdentifier identifier) { + return hiveCatalog.loadTable(identifier); + } + + @Override + public Map loadNamespaceMetadata(Namespace namespace) throws NoSuchNamespaceException { + return hiveCatalog.loadNamespaceMetadata(namespace); + } + + @Override + public boolean dropNamespace(Namespace namespace) throws NamespaceNotEmptyException { + for (TableIdentifier ident : hiveCatalog.listTables(namespace)) { invalidateTable(ident); } - return hiveCatalog.dropNamespace(nmspc); + return hiveCatalog.dropNamespace(namespace); } @Override - public boolean setProperties(Namespace nmspc, Map map) throws NoSuchNamespaceException { - return hiveCatalog.setProperties(nmspc, map); + public boolean setProperties(Namespace namespace, Map map) throws NoSuchNamespaceException { + return hiveCatalog.setProperties(namespace, map); } @Override - public boolean removeProperties(Namespace nmspc, Set set) throws NoSuchNamespaceException { - return hiveCatalog.removeProperties(nmspc, set); + public boolean removeProperties(Namespace namespace, Set set) throws NoSuchNamespaceException { + return hiveCatalog.removeProperties(namespace, set); } @Override @@ -92,6 +599,11 @@ public boolean namespaceExists(Namespace namespace) { return hiveCatalog.namespaceExists(namespace); } + @Override + public Catalog.TableBuilder buildTable(TableIdentifier identifier, Schema schema) { + return hiveCatalog.buildTable(identifier, schema); + } + @Override public List listViews(Namespace namespace) { return hiveCatalog.listViews(namespace); @@ -122,11 +634,6 @@ public void renameView(TableIdentifier from, TableIdentifier to) { hiveCatalog.renameView(from, to); } - @Override - public void invalidateView(TableIdentifier identifier) { - hiveCatalog.invalidateView(identifier); - } - @Override public void initialize(String name, Map properties) { hiveCatalog.initialize(name, properties); diff --git a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCachingCatalogMXBean.java b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCachingCatalogMXBean.java new file mode 100644 index 000000000000..c9ed675f66e7 --- /dev/null +++ b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCachingCatalogMXBean.java @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iceberg.rest; + +/** + * JMX MXBean interface for {@link HMSCachingCatalog} that exposes cache performance statistics. + *

+ * Instances are registered under the object name: + * {@code org.apache.iceberg.rest:type=HMSCachingCatalog,name=<catalogName>}. + *

+ */ +public interface HMSCachingCatalogMXBean { + + /** + * Returns the total number of cache hits (table found in cache and still valid). + * + * @return cache hit count + */ + long getCacheHitCount(); + + /** + * Returns the total number of cache misses (table not found in cache). + * + * @return cache miss count + */ + long getCacheMissCount(); + + /** + * Returns the total number of times a table was loaded from the underlying catalog and stored in cache. + * + * @return cache load count + */ + long getCacheLoadCount(); + + /** + * Returns the total number of times a cached table was invalidated because the actual metadata location differed. + * + * @return cache invalidation count + */ + long getCacheInvalidateCount(); + + /** + * Returns the total number of times a metadata (virtual) table was loaded and cached. + * + * @return cache metadata-table load count + */ + long getCacheMetaLoadCount(); + + /** + * Returns the cache hit rate as a value in the range {@code [0.0, 1.0]}. + * Returns {@code 0.0} when no lookups have been performed. + * + * @return cache hit rate + */ + double getCacheHitRate(); + + /** + * Returns the total number of L1 (short-lived in-memory) cache hits. + * An L1 hit means the table was found in the L2 cache and its L1 TTL had not yet expired, + * so the HMS metadata-location check was skipped entirely. + * + * @return L1 cache hit count + */ + long getL1CacheHitCount(); + + /** + * Returns the total number of L1 cache misses. + * An L1 miss means the table was in the L2 cache but the L1 entry was absent or expired, + * so an HMS metadata-location check was required. + * + * @return L1 cache miss count + */ + long getL1CacheMissCount(); + + /** + * Returns the L1 cache hit rate as a value in the range {@code [0.0, 1.0]}. + * This reflects how often the short-circuit L1 path is taken vs. the full HMS location check. + * Returns {@code 0.0} when no L2-cache-hit lookups have been performed. + * + * @return L1 cache hit rate + */ + double getL1CacheHitRate(); + + /** + * Resets all cache statistics counters to zero. + */ + void resetCacheStats(); +} diff --git a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java index 885e30063528..c6380ec53c5b 100644 --- a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java +++ b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java @@ -241,7 +241,12 @@ private ListNamespacesResponse listNamespaces(Map vars) { } else { namespace = Namespace.empty(); } - return castResponse(ListNamespacesResponse.class, CatalogHandlers.listNamespaces(asNamespaceCatalog, namespace)); + ListNamespacesResponse response = castResponse( + ListNamespacesResponse.class, CatalogHandlers.listNamespaces(asNamespaceCatalog, namespace)); + return ListNamespacesResponse.builder() + .addAll(icebergAuthorizer.filterNamespaces(catalogName, response.namespaces())) + .nextPageToken(response.nextPageToken()) + .build(); } private CreateNamespaceResponse createNamespace(Object body) { @@ -278,7 +283,12 @@ private UpdateNamespacePropertiesResponse updateNamespace(Map va private ListTablesResponse listTables(Map vars) { Namespace namespace = namespaceFromPathVars(vars); - return castResponse(ListTablesResponse.class, CatalogHandlers.listTables(catalog, namespace)); + ListTablesResponse response = + castResponse(ListTablesResponse.class, CatalogHandlers.listTables(catalog, namespace)); + return ListTablesResponse.builder() + .addAll(icebergAuthorizer.filterTables(catalogName, response.identifiers())) + .nextPageToken(response.nextPageToken()) + .build(); } private LoadTableResponse createTable(Map vars, Object body) { @@ -353,14 +363,14 @@ private ListTablesResponse listViews(Map vars) { Namespace namespace = namespaceFromPathVars(vars); String pageToken = PropertyUtil.propertyAsString(vars, "pageToken", null); String pageSize = PropertyUtil.propertyAsString(vars, "pageSize", null); - if (pageSize != null) { - return castResponse( - ListTablesResponse.class, - CatalogHandlers.listViews(asViewCatalog, namespace, pageToken, pageSize)); - } else { - return castResponse( - ListTablesResponse.class, CatalogHandlers.listViews(asViewCatalog, namespace)); - } + ListTablesResponse response = pageSize != null + ? castResponse(ListTablesResponse.class, + CatalogHandlers.listViews(asViewCatalog, namespace, pageToken, pageSize)) + : castResponse(ListTablesResponse.class, CatalogHandlers.listViews(asViewCatalog, namespace)); + return ListTablesResponse.builder() + .addAll(icebergAuthorizer.filterViews(catalogName, response.identifiers())) + .nextPageToken(response.nextPageToken()) + .build(); } private LoadViewResponse createView(Map vars, Object body) { @@ -378,6 +388,8 @@ private RESTResponse viewExists(Map vars) { private LoadViewResponse loadView(Map vars) { TableIdentifier ident = viewIdentFromPathVars(vars); + // Read authorization is enforced by HMS: views are not cached, so loadView always reaches the + // metastore, whose pre-event listener authorizes the read. Checking here would double-authorize. return castResponse(LoadViewResponse.class, CatalogHandlers.loadView(asViewCatalog, ident)); } @@ -416,10 +428,9 @@ private static void commitTransaction(Catalog catalog, CommitTransactionRequest for (UpdateTableRequest tableChange : request.tableChanges()) { Table table = catalog.loadTable(tableChange.identifier()); - if (table instanceof BaseTable) { - Transaction transaction = - Transactions.newTransaction( - tableChange.identifier().toString(), ((BaseTable) table).operations()); + if (table instanceof BaseTable baseTable) { + Transaction transaction = Transactions.newTransaction( + tableChange.identifier().toString(), baseTable.operations()); transactions.add(transaction); BaseTransaction.TransactionTable txTable = @@ -438,85 +449,34 @@ private static void commitTransaction(Catalog catalog, CommitTransactionRequest @SuppressWarnings({"MethodLength", "unchecked"}) private T handleRequest( Route route, Map vars, Object body) { - switch (route) { - case CONFIG: - return (T) config(); - - case LIST_NAMESPACES: - return (T) listNamespaces(vars); - - case CREATE_NAMESPACE: - return (T) createNamespace(body); - - case NAMESPACE_EXISTS: - return (T) namespaceExists(vars); - - case LOAD_NAMESPACE: - return (T) loadNamespace(vars); - - case DROP_NAMESPACE: - return (T) dropNamespace(vars); - - case UPDATE_NAMESPACE: - return (T) updateNamespace(vars, body); - - case LIST_TABLES: - return (T) listTables(vars); - - case CREATE_TABLE: - return (T) createTable(vars, body); - - case DROP_TABLE: - return (T) dropTable(vars); - - case TABLE_EXISTS: - return (T) tableExists(vars); - - case LOAD_TABLE: - return (T) loadTable(vars); - - case REGISTER_TABLE: - return (T) registerTable(vars, body); - - case UPDATE_TABLE: - return (T) updateTable(vars, body); - - case RENAME_TABLE: - return (T) renameTable(body); - - case REPORT_METRICS: - return (T) reportMetrics(vars, body); - - case COMMIT_TRANSACTION: - return (T) commitTransaction(body); - - case LIST_VIEWS: - return (T) listViews(vars); - - case CREATE_VIEW: - return (T) createView(vars, body); - - case VIEW_EXISTS: - return (T) viewExists(vars); - - case LOAD_VIEW: - return (T) loadView(vars); - - case UPDATE_VIEW: - return (T) updateView(vars, body); - - case RENAME_VIEW: - return (T) renameView(body); - - case DROP_VIEW: - return (T) dropView(vars); - - case REGISTER_VIEW: - return (T) registerView(vars, body); - - default: - } - return null; + return switch (route) { + case CONFIG -> (T) config(); + case LIST_NAMESPACES -> (T) listNamespaces(vars); + case CREATE_NAMESPACE -> (T) createNamespace(body); + case NAMESPACE_EXISTS -> (T) namespaceExists(vars); + case LOAD_NAMESPACE -> (T) loadNamespace(vars); + case DROP_NAMESPACE -> (T) dropNamespace(vars); + case UPDATE_NAMESPACE -> (T) updateNamespace(vars, body); + case LIST_TABLES -> (T) listTables(vars); + case CREATE_TABLE -> (T) createTable(vars, body); + case DROP_TABLE -> (T) dropTable(vars); + case TABLE_EXISTS -> (T) tableExists(vars); + case LOAD_TABLE -> (T) loadTable(vars); + case REGISTER_TABLE -> (T) registerTable(vars, body); + case UPDATE_TABLE -> (T) updateTable(vars, body); + case RENAME_TABLE -> (T) renameTable(body); + case REPORT_METRICS -> (T) reportMetrics(vars, body); + case COMMIT_TRANSACTION -> (T) commitTransaction(body); + case LIST_VIEWS -> (T) listViews(vars); + case CREATE_VIEW -> (T) createView(vars, body); + case VIEW_EXISTS -> (T) viewExists(vars); + case LOAD_VIEW -> (T) loadView(vars); + case UPDATE_VIEW -> (T) updateView(vars, body); + case RENAME_VIEW -> (T) renameView(body); + case DROP_VIEW -> (T) dropView(vars); + case REGISTER_VIEW -> (T) registerView(vars, body); + default -> null; + }; } diff --git a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogFactory.java b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogFactory.java index 6f1694246261..805ad733be93 100644 --- a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogFactory.java +++ b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogFactory.java @@ -16,6 +16,7 @@ * specific language governing permissions and limitations * under the License. */ + package org.apache.iceberg.rest; import java.lang.reflect.InvocationTargetException; @@ -69,9 +70,26 @@ public String getPath() { /** * Creates the catalog instance. + * @param authorizer authorizes reads served from the table cache; the caching catalog enforces it + * on cache hits, which never reach HMS. Ignored when caching is disabled, as an + * uncached load reaches HMS and is authorized there. * @return the catalog */ - private Catalog createCatalog() { + private Catalog createCatalog(IcebergAuthorizer authorizer) { + final HiveCatalog hiveCatalog = createHiveCatalog(configuration); + long expiry = MetastoreConf.getLongVar(configuration, MetastoreConf.ConfVars.ICEBERG_CATALOG_CACHE_EXPIRY); + return expiry > 0 ? new HMSCachingCatalog(hiveCatalog, expiry, authorizer) : hiveCatalog; + } + + /** + * Builds the underlying {@link HiveCatalog} from the given configuration. + *

Exposed so tests can obtain a catalog through the exact production construction path rather + * than duplicating it; the servlet path wraps the result in an {@link HMSCachingCatalog} when a + * positive cache expiry is configured (see {@link #createCatalog()}).

+ * @param configuration the configuration + * @return the initialized HiveCatalog + */ + public static HiveCatalog createHiveCatalog(Configuration configuration) { final Map properties = new TreeMap<>(); final String configUri = MetastoreConf.getVar(configuration, MetastoreConf.ConfVars.THRIFT_URIS); // Clear THRIFT_URIS so HiveCatalog doesn't accidentally use Thrift connection @@ -101,8 +119,7 @@ private Catalog createCatalog() { hiveCatalog.setConf(configuration); final String catalogName = MetastoreConf.getVar(configuration, MetastoreConf.ConfVars.CATALOG_DEFAULT); hiveCatalog.initialize(catalogName, properties); - long expiry = MetastoreConf.getLongVar(configuration, MetastoreConf.ConfVars.ICEBERG_CATALOG_CACHE_EXPIRY); - return expiry > 0 ? new HMSCachingCatalog(hiveCatalog, expiry) : hiveCatalog; + return hiveCatalog; } /** @@ -110,13 +127,12 @@ private Catalog createCatalog() { * @param catalog the Iceberg catalog * @return the servlet */ - private HttpServlet createServlet(Catalog catalog) { + private HttpServlet createServlet(Catalog catalog, IcebergAuthorizer icebergAuthorizer) { String authType = MetastoreConf.getVar(configuration, ConfVars.CATALOG_SERVLET_AUTH); // Iceberg REST client uses "catalog" by default List scopes = Collections.singletonList("catalog"); ServletSecurity security = new ServletSecurity(AuthType.fromString(authType), configuration, req -> scopes); String catalogName = MetastoreConf.getVar(configuration, ConfVars.CATALOG_DEFAULT); - IcebergAuthorizer icebergAuthorizer = new IcebergAuthorizer(configuration); List reporters = createReporters(); var adapter = new HMSCatalogAdapter(catalogName, catalog, icebergAuthorizer, reporters); return security.proxy(new HMSCatalogServlet(adapter)); @@ -140,7 +156,10 @@ private List createReporters() { */ private HttpServlet createServlet() { if (port >= 0 && path != null && !path.isEmpty()) { - return createServlet(createCatalog()); + // Build the authorizer first so it can be shared: the caching catalog uses it to authorize + // cache hits, and the adapter uses it for list filtering and stage-create. + IcebergAuthorizer icebergAuthorizer = new IcebergAuthorizer(configuration); + return createServlet(createCatalog(icebergAuthorizer), icebergAuthorizer); } return null; } diff --git a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java index 92cd4af484a1..081151090ab5 100644 --- a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java +++ b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java @@ -29,6 +29,7 @@ import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.rest.HMSCatalogAdapter.Route; import org.apache.iceberg.rest.HTTPRequest.HTTPMethod; +import org.apache.iceberg.exceptions.RESTException; import org.apache.iceberg.rest.responses.ErrorResponse; import org.apache.iceberg.util.Pair; import org.slf4j.Logger; @@ -80,7 +81,13 @@ protected void service(HttpServletRequest request, HttpServletResponse response) if (responseBody != null) { RESTObjectMapper.mapper().writeValue(response.getWriter(), responseBody); } + } catch (RESTException e) { + // A RESTException is thrown by HMSCatalogAdapter.execute() after the error handler has + // already written the correct HTTP status and body to the response (e.g. 404, 403). + // It is not an unexpected server failure, so log at DEBUG to avoid flooding the console. + LOG.debug("REST request resulted in a client error (already handled): {}", e.getMessage()); } catch (RuntimeException | IOException e) { + // Genuine unexpected server error – log the full stack trace. LOG.error("Error processing REST request", e); response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } diff --git a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java index 2df051105b77..c01027dc57de 100644 --- a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java +++ b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/IcebergAuthorizer.java @@ -9,11 +9,12 @@ * * http://www.apache.org/licenses/LICENSE-2.0 * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. */ package org.apache.iceberg.rest; @@ -22,8 +23,10 @@ import static org.apache.iceberg.hive.HiveCatalog.HMS_DB_OWNER_TYPE; import static org.apache.iceberg.hive.HiveCatalog.HMS_TABLE_OWNER; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.function.Supplier; @@ -35,8 +38,10 @@ import org.apache.hadoop.hive.metastore.conf.MetastoreConf; import org.apache.hadoop.hive.ql.metadata.HiveException; import org.apache.hadoop.hive.ql.metadata.HiveUtils; +import org.apache.hadoop.hive.ql.security.HiveAuthenticationProvider; import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAccessControlException; import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthorizer; +import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthorizerFactory; import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthzContext; import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthzPluginException; import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthzSessionContext; @@ -44,7 +49,9 @@ import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveOperationType; import org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObject; import org.apache.hadoop.hive.ql.security.authorization.plugin.metastore.HiveMetaStoreAuthorizer; +import org.apache.iceberg.MetadataTableType; import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.ForbiddenException; import org.apache.iceberg.hive.HiveHadoopUtil; import org.apache.iceberg.rest.requests.CreateTableRequest; @@ -77,27 +84,69 @@ class IcebergAuthorizer { .formatted(Arrays.toString(classes))); } - this.authorizerSupplier = () -> { + // Building a HiveAuthorizer is dominated by identity-independent work: HiveConf.cloneConf (a full + // Configuration deep-copy) and the reflective factory/authenticator lookups. Since read + // authorization now runs on every table cache hit, do that work once per thread and keep only the + // identity-sensitive steps per call. This mirrors HiveMetaStoreAuthorizer, which caches the + // authenticator in a ThreadLocal and refreshes it via setConf on every event; we additionally cache + // the cloned conf and factory (which it rebuilds per event) since they carry no identity. + // + // Per-thread caching is required for correctness, not just speed: the default authenticator + // (HadoopDefaultAuthenticator) resolves and caches the user name at setConf time from the current + // UserGroupInformation. Jetty worker threads are pooled and reused across end users, so the toolkit + // must be thread-confined and setConf must be re-run each call to bind the current request's identity. + // A single shared authorizer would pin authorization to whichever user built it first. + final ThreadLocal toolkit = ThreadLocal.withInitial(() -> { try { final var hiveConf = HiveConf.cloneConf(conf); final var authorizerFactory = HiveUtils.getAuthorizerFactory(hiveConf, HiveConf.ConfVars.HIVE_AUTHORIZATION_MANAGER); - final var authenticator = HiveUtils.getAuthenticator(hiveConf, HiveConf.ConfVars.HIVE_METASTORE_AUTHENTICATOR_MANAGER); - authenticator.setConf(hiveConf); - - final var authzContextBuilder = new HiveAuthzSessionContext.Builder(); - authzContextBuilder.setClientType(HiveAuthzSessionContext.CLIENT_TYPE.HIVEMETASTORE); - authzContextBuilder.setSessionString("IcebergRESTCatalog"); - return authorizerFactory.createHiveAuthorizer( - new HiveMetastoreClientFactoryImpl(hiveConf), hiveConf, authenticator, authzContextBuilder.build()); + return new AuthorizerToolkit(hiveConf, authorizerFactory, authenticator); } catch (HiveException e) { throw new IllegalStateException("Failed to initialize Hive authorizer for Iceberg REST Catalog", e); } - }; + }); + + this.authorizerSupplier = () -> newRequestAuthorizer(toolkit.get()); + } + + /** + * Builds a request-scoped {@link HiveAuthorizer} from the calling thread's cached {@link + * AuthorizerToolkit}. Rebinds the authenticator to the current request's UGI via {@code setConf} + * and rebuilds the authorizer on every call (rather than caching it), matching {@link + * HiveMetaStoreAuthorizer#createHiveMetaStoreAuthorizer()}, so no authorizer implementation can + * retain a stale identity across a pooled thread's successive requests. + * + * @param kit the calling thread's identity-independent building blocks + * @return an authorizer bound to the current request's identity + * @throws IllegalStateException if the authorization plugin fails to initialize + */ + private static HiveAuthorizer newRequestAuthorizer(AuthorizerToolkit kit) { + try { + kit.authenticator.setConf(kit.hiveConf); + final var authzContextBuilder = new HiveAuthzSessionContext.Builder(); + authzContextBuilder.setClientType(HiveAuthzSessionContext.CLIENT_TYPE.HIVEMETASTORE); + authzContextBuilder.setSessionString("IcebergRESTCatalog"); + return kit.authorizerFactory.createHiveAuthorizer( + new HiveMetastoreClientFactoryImpl(kit.hiveConf), kit.hiveConf, kit.authenticator, + authzContextBuilder.build()); + } catch (HiveException e) { + throw new IllegalStateException("Failed to initialize Hive authorizer for Iceberg REST Catalog", e); + } } + /** + * Per-thread, identity-independent building blocks for a {@link HiveAuthorizer}. Cached in a + * {@link ThreadLocal} so the expensive {@link HiveConf} clone and reflective factory/authenticator + * lookups run once per thread; the identity-sensitive {@code setConf}/{@code createHiveAuthorizer} + * steps still run on every authorization call. + */ + private record AuthorizerToolkit(HiveConf hiveConf, + HiveAuthorizerFactory authorizerFactory, + HiveAuthenticationProvider authenticator) {} + @VisibleForTesting IcebergAuthorizer(Supplier authorizerSupplier) { this.authorizerSupplier = authorizerSupplier; @@ -151,14 +200,165 @@ void validateStageCreateTable(String catalogName, Namespace namespace, Map filterTables(String catalogName, List identifiers) { + return filterTableOrViews(catalogName, identifiers, "show tables"); + } + + /** + * Filters a view listing down to the entries the user may see. See {@link #filterTables}. + * + * @param catalogName the Hive catalog name + * @param identifiers the full listing to filter + * @return the subset the user is allowed to see, sorted by name + * @throws IllegalStateException if the authorization plugin fails + */ + List filterViews(String catalogName, List identifiers) { + return filterTableOrViews(catalogName, identifiers, "show views"); + } + + private List filterTableOrViews(String catalogName, List identifiers, + String commandString) { + var authorizer = authorizerSupplier.get(); + if (authorizer == null) { + return identifiers; + } + List objects = new ArrayList<>(identifiers.size()); + for (TableIdentifier identifier : identifiers) { + objects.add(tableOrView(catalogName, identifier)); + } + List allowed = filterListCmd(authorizer, objects, commandString); + List result = new ArrayList<>(allowed.size()); + for (HivePrivilegeObject object : allowed) { + result.add(TableIdentifier.of(object.getDbname(), object.getObjectName())); + } + result.sort(Comparator.comparing(TableIdentifier::name)); + return result; + } + + /** + * Filters a namespace listing down to the databases the user may see, mirroring how Hive filters + * a {@code SHOW DATABASES} result. Hive Metastore performs no pre-event authorization for + * {@code get_databases}, so without this filter a user would see databases they cannot access. + * Multi-level namespaces cannot map to a Hive database and are dropped rather than failing the + * whole listing. + * + * @param catalogName the Hive catalog name + * @param namespaces the full listing to filter + * @return the subset the user is allowed to see, sorted by name + * @throws IllegalStateException if the authorization plugin fails + */ + List filterNamespaces(String catalogName, List namespaces) { + var authorizer = authorizerSupplier.get(); + if (authorizer == null) { + return namespaces; + } + // Only single-level namespaces map to a Hive database; multi-level ones are dropped. + List objects = new ArrayList<>(namespaces.size()); + for (Namespace namespace : namespaces) { + if (namespace.levels().length == 1) { + objects.add(database(catalogName, namespace)); + } + } + List allowed = filterListCmd(authorizer, objects, "show databases"); + List result = new ArrayList<>(allowed.size()); + for (HivePrivilegeObject object : allowed) { + result.add(Namespace.of(object.getDbname())); + } + result.sort(Comparator.comparing(namespace -> namespace.level(0))); + return result; + } + + private List filterListCmd(HiveAuthorizer authorizer, List objects, + String commandString) { + var builder = new HiveAuthzContext.Builder(); + builder.setCommandString(commandString); + try { + List allowed = authorizer.filterListCmdObjects(objects, builder.build()); + return allowed == null ? List.of() : allowed; + } catch (HiveAccessControlException e) { + throw new ForbiddenException(e, e.getMessage()); + } catch (HiveAuthzPluginException e) { + throw new IllegalStateException("Failed to filter " + commandString + " results", e); + } + } + + /** + * Normalizes a metadata-table identifier ({@code db.table.}) to its base table + * ({@code db.table}) so access cannot be granted through a metadata-table name. Non-metadata + * identifiers are returned unchanged. + */ + private static TableIdentifier baseTableIdentifier(TableIdentifier identifier) { + String[] levels = identifier.namespace().levels(); + // A metadata-table identifier is db.table., so its namespace always carries at least the + // parent database and table. A single-level namespace whose name happens to match a metadata + // type is a real table, not a metadata table, and must be left untouched. + if (levels.length >= 2 && MetadataTableType.from(identifier.name()) != null) { + return TableIdentifier.of(levels); + } + return identifier; + } + + private static HivePrivilegeObject tableOrView(String catalogName, TableIdentifier identifier) { + return new HivePrivilegeObject(HivePrivilegeObject.HivePrivilegeObjectType.TABLE_OR_VIEW, catalogName, + identifier.namespace().level(0), identifier.name()); + } + + private static HivePrivilegeObject database(String catalogName, Namespace namespace) { + Preconditions.checkArgument(namespace.levels().length == 1, "Hive does not support multi-level namespaces"); + return new HivePrivilegeObject(HivePrivilegeObject.HivePrivilegeObjectType.DATABASE, catalogName, + namespace.level(0), (String) null); + } + + private void check(HiveOperationType operation, List inputs, + List outputs, String commandString) { + check(authorizerSupplier.get(), operation, inputs, outputs, commandString); + } + + private void check(HiveAuthorizer authorizer, HiveOperationType operation, List inputs, + List outputs, String commandString) { + if (authorizer == null) { + LOG.debug("No pre-event listener is configured, skipping {} authorization", operation); + return; + } var builder = new HiveAuthzContext.Builder(); - builder.setCommandString("create table " + request.name()); + builder.setCommandString(commandString); try { - authorizer.checkPrivileges(HiveOperationType.CREATETABLE, inputs, outputs, builder.build()); + authorizer.checkPrivileges(operation, inputs, outputs, builder.build()); } catch (HiveAccessControlException e) { throw new ForbiddenException(e, e.getMessage()); } catch (HiveAuthzPluginException e) { - throw new IllegalStateException("Failed to check privileges stage-create", e); + throw new IllegalStateException("Failed to check privileges for " + operation, e); } } } diff --git a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/BaseRESTCatalogTests.java b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/BaseRESTCatalogTests.java index 206d72c9cb1b..4d2e0a5506c9 100644 --- a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/BaseRESTCatalogTests.java +++ b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/BaseRESTCatalogTests.java @@ -125,8 +125,9 @@ void testPermissionsWithDeniedUser() throws Exception { throw new AssertionError("Catalog operation failed", e); } try (var client = RCKUtils.initCatalogClient(properties.get())) { - // Should this fail? - Assertions.assertTrue(client.listNamespaces().contains(db)); + // List operations are result-filtered: a user who can read nothing sees an empty listing + // rather than an error. + Assertions.assertFalse(client.listNamespaces().contains(db)); testUnauthorizedAccess(() -> client.namespaceExists(db)); testUnauthorizedAccess(() -> client.loadNamespaceMetadata(db)); testUnauthorizedAccess(() -> client.createNamespace(Namespace.of("new-db"))); @@ -134,16 +135,14 @@ void testPermissionsWithDeniedUser() throws Exception { testUnauthorizedAccess(() -> client.setProperties(db, Collections.singletonMap("key", "value"))); testUnauthorizedAccess(() -> client.removeProperties(db, Collections.singleton("key"))); - // Should this fail? - Assertions.assertEquals(Collections.singletonList(table), client.listTables(db)); + Assertions.assertTrue(client.listTables(db).isEmpty()); testUnauthorizedAccess(() -> client.tableExists(table)); testUnauthorizedAccess(() -> client.loadTable(table)); testUnauthorizedAccess(() -> client.createTable(TableIdentifier.of(db, "new-table"), new Schema())); testUnauthorizedAccess(() -> client.renameTable(table, TableIdentifier.of(db, "new-table"))); testUnauthorizedAccess(() -> client.dropTable(table)); - // Should this fail? - Assertions.assertEquals(Collections.singletonList(view), client.listViews(db)); + Assertions.assertTrue(client.listViews(db).isEmpty()); testUnauthorizedAccess(() -> client.viewExists(view)); testUnauthorizedAccess(() -> client.loadView(view)); testUnauthorizedAccess(() -> client.buildView(TableIdentifier.of(db, "new-view")) @@ -184,7 +183,7 @@ void testPermissionsWithReadOnlyUser() throws Exception { throw new AssertionError("Catalog operation failed", e); } try (var client = RCKUtils.initCatalogClient(properties.get())) { - // Should this fail? + // A read-only user can read, so result-filtered listings still show what they may read. Assertions.assertTrue(client.listNamespaces().contains(db)); Assertions.assertTrue(client.namespaceExists(db)); Assertions.assertNotNull(client.loadNamespaceMetadata(db)); @@ -193,7 +192,6 @@ void testPermissionsWithReadOnlyUser() throws Exception { testUnauthorizedAccess(() -> client.setProperties(db, Collections.singletonMap("key", "value"))); testUnauthorizedAccess(() -> client.removeProperties(db, Collections.singleton("key"))); - // Should this fail? Assertions.assertEquals(Collections.singletonList(table), client.listTables(db)); Assertions.assertTrue(client.tableExists(table)); Assertions.assertNotNull(client.loadTable(table)); @@ -201,7 +199,6 @@ void testPermissionsWithReadOnlyUser() throws Exception { testUnauthorizedAccess(() -> client.renameTable(table, TableIdentifier.of(db, "new-table"))); testUnauthorizedAccess(() -> client.dropTable(table)); - // Should this fail? Assertions.assertEquals(Collections.singletonList(view), client.listViews(db)); Assertions.assertTrue(client.viewExists(view)); Assertions.assertNotNull(client.loadView(view)); diff --git a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestHMSCachingCatalogCache.java b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestHMSCachingCatalogCache.java new file mode 100644 index 000000000000..267f455d0cf9 --- /dev/null +++ b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestHMSCachingCatalogCache.java @@ -0,0 +1,249 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iceberg.rest; + +import org.apache.hadoop.hive.metastore.ServletSecurity.AuthType; +import org.apache.hadoop.hive.metastore.annotation.MetastoreCheckinTest; +import org.apache.hadoop.hive.metastore.conf.MetastoreConf; +import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAccessControlException; +import org.apache.hadoop.hive.ql.security.authorization.plugin.HiveAuthorizer; +import org.apache.iceberg.DataFile; +import org.apache.iceberg.DataFiles; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.Table; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.exceptions.ForbiddenException; +import org.apache.iceberg.exceptions.NoSuchTableException; +import org.apache.iceberg.hive.HiveCatalog; +import org.apache.iceberg.rest.extension.HiveRESTCatalogServerExtension; +import org.junit.experimental.categories.Category; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.RegisterExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * Tests the two-level table caching behaviour of {@link HMSCachingCatalog}: L2 (Caffeine) hits + * return the same instance, explicit invalidation and drop evict the cache, the L1 recency guard + * can be disabled, and a table dropped underneath the cache reports not-found rather than serving + * a stale instance. + */ +@Category(MetastoreCheckinTest.class) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class TestHMSCachingCatalogCache { + + private static final long CACHE_EXPIRY_MS = 5 * 60 * 1_000L; + private static final String NS = "cache_test_ns"; + private static final Namespace NAMESPACE = Namespace.of(NS); + private static final String TABLE = "cache_test_table"; + private static final TableIdentifier TABLE_ID = TableIdentifier.of(NAMESPACE, TABLE); + private static final Schema SCHEMA = new Schema(); + + @RegisterExtension + private static final HiveRESTCatalogServerExtension SERVER = + HiveRESTCatalogServerExtension.builder(AuthType.NONE) + .configure(MetastoreConf.ConfVars.ICEBERG_CATALOG_CACHE_EXPIRY.getVarname(), + String.valueOf(CACHE_EXPIRY_MS)) + .configure("hive.in.test", "true") + .build(); + + private HiveCatalog hiveCatalog; + private HMSCachingCatalog catalog; + + @BeforeAll + void setupAll() { + hiveCatalog = SERVER.newServerCatalog(); + } + + @BeforeEach + void setupEach() { + catalog = new HMSCachingCatalog(hiveCatalog, CACHE_EXPIRY_MS); + hiveCatalog.createNamespace(NAMESPACE); + } + + @AfterEach + void cleanup() { + try { hiveCatalog.dropTable(TABLE_ID, false); } catch (Exception ignored) {} + try { hiveCatalog.dropNamespace(NAMESPACE); } catch (Exception ignored) {} + // Do not call catalog.close() — it would unregister the JMX MBean that other test + // classes (e.g. TestHMSCachingCatalogStats) rely on via the server-side catalog. + } + + @Test + void testL2CacheReturnsSameTableInstance() { + hiveCatalog.createTable(TABLE_ID, SCHEMA); + + Table first = catalog.loadTable(TABLE_ID); + Table second = catalog.loadTable(TABLE_ID); + + // Caffeine stores object references; a cache hit returns the identical instance. + assertThat(first).isSameAs(second); + } + + @Test + void testInvalidateTableForcesReload() { + hiveCatalog.createTable(TABLE_ID, SCHEMA); + + Table before = catalog.loadTable(TABLE_ID); + assertThat(before.currentSnapshot()).isNull(); // fresh table, no snapshot yet + + // Advance the underlying table's metadata location by committing a new snapshot. + Table raw = hiveCatalog.loadTable(TABLE_ID); + DataFile file = DataFiles.builder(PartitionSpec.unpartitioned()) + .withPath(raw.location() + "/data/file-0.parquet") + .withFileSizeInBytes(1024).withRecordCount(1).build(); + raw.newAppend().appendFile(file).commit(); + + // Explicit invalidation evicts both the L2 and L1 caches. + catalog.invalidateTable(TABLE_ID); + + Table after = catalog.loadTable(TABLE_ID); + assertThat(after).isNotSameAs(before); + assertThat(after.currentSnapshot()).isNotNull(); + assertThat(after.currentSnapshot().snapshotId()).isEqualTo(raw.currentSnapshot().snapshotId()); + } + + @Test + void testDropTableEvictsCache() { + // Create a table with a snapshot so the cached version has observable state. + hiveCatalog.createTable(TABLE_ID, SCHEMA); + Table raw = hiveCatalog.loadTable(TABLE_ID); + DataFile file = DataFiles.builder(PartitionSpec.unpartitioned()) + .withPath(raw.location() + "/data/file-0.parquet") + .withFileSizeInBytes(1024).withRecordCount(1).build(); + raw.newAppend().appendFile(file).commit(); + + Table cached = catalog.loadTable(TABLE_ID); + assertThat(cached.currentSnapshot()).isNotNull(); + + // Drop clears both the L2 and L1 caches via invalidateTable. + catalog.dropTable(TABLE_ID); + + // Recreate a fresh empty table (no snapshot) and reload. + hiveCatalog.createTable(TABLE_ID, SCHEMA); + + Table reloaded = catalog.loadTable(TABLE_ID); + assertThat(reloaded).isNotSameAs(cached); + assertThat(reloaded.currentSnapshot()).isNull(); + } + + @Test + void testLoadTableWithL1CacheDisabled() { + Table created = hiveCatalog.createTable(TABLE_ID, SCHEMA); + + // Disable the L1 recency guard; its backing map is then an immutable empty map, so any write + // to it would throw. The second load exercises the L2-hit path that records L1 freshness. + var conf = hiveCatalog.getConf(); + int prevSize = conf.getInt("hms.caching.catalog.l1.cache.size", 32); + int prevTtl = conf.getInt("hms.caching.catalog.l1.cache.ttl", 3_000); + conf.setInt("hms.caching.catalog.l1.cache.size", 0); + try { + HMSCachingCatalog noL1 = new HMSCachingCatalog(hiveCatalog, CACHE_EXPIRY_MS); + Table first = noL1.loadTable(TABLE_ID); + Table second = noL1.loadTable(TABLE_ID); + assertThat(first.location()).isEqualTo(created.location()); + assertThat(second.location()).isEqualTo(created.location()); + } finally { + conf.setInt("hms.caching.catalog.l1.cache.size", prevSize); + conf.setInt("hms.caching.catalog.l1.cache.ttl", prevTtl); + } + } + + @Test + void testReloadOfDroppedTableThrowsNoSuchTable() { + hiveCatalog.createTable(TABLE_ID, SCHEMA); + + // Disable the L1 recency guard so the second load re-checks the HMS location instead of + // short-circuiting on L1 freshness. + var conf = hiveCatalog.getConf(); + int prevSize = conf.getInt("hms.caching.catalog.l1.cache.size", 32); + conf.setInt("hms.caching.catalog.l1.cache.size", 0); + try { + HMSCachingCatalog noL1 = new HMSCachingCatalog(hiveCatalog, CACHE_EXPIRY_MS); + + // Warm the L2 cache with the table. + Table loaded = noL1.loadTable(TABLE_ID); + assertThat(loaded).isNotNull(); + + // Drop the table straight through the underlying catalog so noL1's L2 entry is left stale. + hiveCatalog.dropTable(TABLE_ID, false); + + // Reloading must not serve the ghost: the null HMS location evicts the entry and signals + // not-found. + assertThatThrownBy(() -> noL1.loadTable(TABLE_ID)) + .isInstanceOf(NoSuchTableException.class); + } finally { + conf.setInt("hms.caching.catalog.l1.cache.size", prevSize); + } + } + + @Test + void testCacheHitEnforcesAuthorization() throws Exception { + hiveCatalog.createTable(TABLE_ID, SCHEMA); + + // A denying authorizer: any privilege check throws, mapped to ForbiddenException. + HiveAuthorizer hiveAuthorizer = mock(HiveAuthorizer.class); + doThrow(new HiveAccessControlException("access denied")) + .when(hiveAuthorizer).checkPrivileges(any(), anyList(), anyList(), any()); + HMSCachingCatalog authzCatalog = + new HMSCachingCatalog(hiveCatalog, CACHE_EXPIRY_MS, new IcebergAuthorizer(() -> hiveAuthorizer)); + + // Cold miss reloads through HMS (authorized there), so the cache authorizer is NOT consulted. + Table firstLoad = authzCatalog.loadTable(TABLE_ID); + assertThat(firstLoad).isNotNull(); + verify(hiveAuthorizer, never()).checkPrivileges(any(), anyList(), anyList(), any()); + + // The next load is served from the L1 cache without reaching HMS, so it must be authorized here + // — the denying authorizer turns the hit into a ForbiddenException. + assertThatThrownBy(() -> authzCatalog.loadTable(TABLE_ID)).isInstanceOf(ForbiddenException.class); + verify(hiveAuthorizer, times(1)).checkPrivileges(any(), anyList(), anyList(), any()); + } + + @Test + void testCacheHitAllowedByAuthorizer() { + Table created = hiveCatalog.createTable(TABLE_ID, SCHEMA); + + // A permissive authorizer: checkPrivileges is a no-op mock, so it never throws. + HiveAuthorizer hiveAuthorizer = mock(HiveAuthorizer.class); + HMSCachingCatalog authzCatalog = + new HMSCachingCatalog(hiveCatalog, CACHE_EXPIRY_MS, new IcebergAuthorizer(() -> hiveAuthorizer)); + + Table first = authzCatalog.loadTable(TABLE_ID); + Table second = authzCatalog.loadTable(TABLE_ID); + assertThat(first.location()).isEqualTo(created.location()); + // The second load is an authorized cache hit and returns the identical instance. + assertThat(second).isSameAs(first); + } +} diff --git a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestHMSCachingCatalogStats.java b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestHMSCachingCatalogStats.java new file mode 100644 index 000000000000..476134afa747 --- /dev/null +++ b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestHMSCachingCatalogStats.java @@ -0,0 +1,284 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.iceberg.rest; + +import java.lang.management.ManagementFactory; +import java.util.Set; + +import javax.management.MBeanServer; +import javax.management.ObjectName; + +import org.apache.hadoop.hive.metastore.ServletSecurity.AuthType; +import org.apache.hadoop.hive.metastore.annotation.MetastoreCheckinTest; +import org.apache.hadoop.hive.metastore.conf.MetastoreConf; +import org.apache.iceberg.DataFile; +import org.apache.iceberg.DataFiles; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.Table; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.hive.HiveCatalog; +import org.apache.iceberg.rest.extension.HiveRESTCatalogServerExtension; +import org.junit.experimental.categories.Category; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.extension.RegisterExtension; + +/** + * Component tests that verify the {@link HMSCachingCatalog} cache-statistics counters + * (hit, miss, load, invalidate, l1-hit, l1-miss, and their rates) are updated correctly + * and exposed accurately via both the getters and the JMX MBean registered under + * {@code org.apache.iceberg.rest:type=HMSCachingCatalog,name=*}. + * + *

Each test drives a freshly built {@link HMSCachingCatalog} directly (obtained through the + * server extension, which wraps a {@link HiveCatalog} built via the production + * {@link org.apache.iceberg.rest.HMSCatalogFactory} path). A fresh instance starts with all + * counters at zero, so assertions use absolute values rather than deltas.

+ * + *

The server is started with {@link AuthType#NONE} so the tests focus purely on + * caching behaviour without any authentication noise.

+ */ +@Category(MetastoreCheckinTest.class) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class TestHMSCachingCatalogStats { + + /** 5 minutes expressed in milliseconds – the value injected into {@code ICEBERG_CATALOG_CACHE_EXPIRY}. */ + private static final long CACHE_EXPIRY_MS = 5 * 60 * 1_000L; + private static final String NS = "caching_stats_test_db"; + private static final Namespace NAMESPACE = Namespace.of(NS); + private static final String TABLE = "caching_stats_test_table"; + private static final TableIdentifier TABLE_ID = TableIdentifier.of(NAMESPACE, TABLE); + + @RegisterExtension + private static final HiveRESTCatalogServerExtension SERVER = HiveRESTCatalogServerExtension.builder(AuthType.NONE) + // Without a positive expiry the HMSCatalogFactory skips HMSCachingCatalog entirely. + .configure(MetastoreConf.ConfVars.ICEBERG_CATALOG_CACHE_EXPIRY.getVarname(), String.valueOf(CACHE_EXPIRY_MS)) + .configure("hive.in.test", "true").build(); + + /** Underlying catalog, shared across tests; used for setup and direct (uncached) mutations. */ + private HiveCatalog hiveCatalog; + /** The caching catalog under test; rebuilt fresh for every test so counters start at zero. */ + private HMSCachingCatalog catalog; + /** The platform {@link MBeanServer} used for JMX-based assertions. */ + private MBeanServer mbs; + /** The JMX ObjectName registered by the current {@link #catalog} instance. */ + private ObjectName jmxObjectName; + + @BeforeAll + void setupAll() { + hiveCatalog = SERVER.newServerCatalog(); + } + + @BeforeEach + void setupEach() throws Exception { + catalog = new HMSCachingCatalog(hiveCatalog, CACHE_EXPIRY_MS); + hiveCatalog.createNamespace(NAMESPACE); + + // Resolve the JMX ObjectName registered by the catalog instance just created. We use a + // wildcard so the test is independent of the exact catalog name. + mbs = ManagementFactory.getPlatformMBeanServer(); + Set names = mbs.queryNames( + new ObjectName("org.apache.iceberg.rest:type=HMSCachingCatalog,*"), null); + Assertions.assertFalse(names.isEmpty(), + "HMSCachingCatalog MBean must be registered in the platform MBeanServer"); + jmxObjectName = names.iterator().next(); + } + + /** Remove any namespace/table created by the test so each run starts clean. */ + @AfterEach + void cleanup() { + try { + hiveCatalog.dropTable(TABLE_ID, false); + } catch (Exception ignored) { + // table may not exist + } + try { + hiveCatalog.dropNamespace(NAMESPACE); + } catch (Exception ignored) { + // namespace may not exist + } + } + + // --------------------------------------------------------------------------- + // JMX helpers + // --------------------------------------------------------------------------- + + private long jmxLong(String attribute) throws Exception { + return (long) mbs.getAttribute(jmxObjectName, attribute); + } + + private double jmxDouble(String attribute) throws Exception { + return (double) mbs.getAttribute(jmxObjectName, attribute); + } + + private void invokeJmxOperation(String operationName) throws Exception { + mbs.invoke(jmxObjectName, operationName, new Object[0], new String[0]); + } + + // --------------------------------------------------------------------------- + // tests + // --------------------------------------------------------------------------- + + /** + * Verifies that the {@link HMSCachingCatalog} correctly tracks cache hits, misses, + * loads, invalidations, L1 hits, and L1 misses. + * + *

Counter states for the four {@code loadTable} calls: + *

+   *   Call 1 – cold L2 miss : onCacheMiss  + onCacheLoad               → miss=1, load=1
+   *   Call 2 – L1 hit       : onL1CacheHit + onCacheHit                 → l1Hit=1, hit=1
+   *   Call 3 – L1 hit       : onL1CacheHit + onCacheHit                 → l1Hit=2, hit=2
+   *   [sleep >L1 TTL; mutated table has new METADATA_LOCATION in HMS]
+   *   Call 4 – L1 expired,
+   *            location mismatch: onL1CacheMiss + onCacheInvalidate
+   *                             + onCacheLoad                           → l1Miss=1, invalidate=1, load=2
+   * 
+ * Note: call 4 does NOT fire {@code onCacheMiss}: that counter only increments when the L2 + * {@code getIfPresent} returns null (the else-branch). The location-mismatch path goes through the + * if-branch, evicts L2 internally, and falls straight to {@code tableCache.get} + {@code onCacheLoad}. + */ + @Test + void testCacheCountersAreUpdated() throws Exception { + Table created = hiveCatalog.createTable(TABLE_ID, new Schema()); + + // First load → cache miss + load; must return the table we just created. + Table firstLoad = catalog.loadTable(TABLE_ID); + Assertions.assertEquals(created.location(), firstLoad.location(), + "First load must return the table we just created"); + // Second load → L1 hit (within TTL, HMS location check skipped) + catalog.loadTable(TABLE_ID); + // Third load → L1 hit + catalog.loadTable(TABLE_ID); + + // Mutate the table by appending a data file – this creates a new snapshot which advances + // METADATA_LOCATION in HMS, so the next loadTable call through the caching catalog will detect + // the stale cached location and invalidate it. + Table table = hiveCatalog.loadTable(TABLE_ID); + DataFile dataFile = DataFiles.builder(PartitionSpec.unpartitioned()) + .withPath(table.location() + "/data/fake-0.parquet") + .withFileSizeInBytes(1024).withRecordCount(1).build(); + table.newAppend().appendFile(dataFile).commit(); + + // Default L1 TTL is 3 000 ms; sleep 3 500 ms to ensure the entry is expired. + Thread.sleep(3_500); + // Fourth load → L1 miss + cache invalidation + reload + Table reloaded = catalog.loadTable(TABLE_ID); + + // -- counter assertions (exact values; see Javadoc above for derivation) -- + Assertions.assertEquals(1L, catalog.getCacheMissCount(), + "Expected exactly 1 cache miss (cold load on call 1)"); + Assertions.assertEquals(2L, catalog.getCacheLoadCount(), + "Expected exactly 2 cache loads (call 1 + post-invalidation call 4)"); + Assertions.assertEquals(2L, catalog.getCacheHitCount(), + "Expected exactly 2 cache hits (calls 2 and 3)"); + Assertions.assertEquals(1L, catalog.getCacheInvalidateCount(), + "Expected exactly 1 cache invalidation (metadata location changed on call 4)"); + Assertions.assertEquals(2L, catalog.getL1CacheHitCount(), + "Expected exactly 2 L1 hits (calls 2 and 3, within TTL)"); + Assertions.assertEquals(1L, catalog.getL1CacheMissCount(), + "Expected exactly 1 L1 miss (call 4, after TTL expiry)"); + + // The reloaded table must reflect the new snapshot created by the append above; + // this confirms the staleness-detection path returned fresh data, not the stale cache entry. + Assertions.assertNotNull(reloaded.currentSnapshot(), + "Staleness detection must have reloaded the table with its new snapshot"); + + // Rate attributes must be valid ratios in (0.0, 1.0]. + double hitRate = catalog.getCacheHitRate(); + Assertions.assertTrue(hitRate > 0.0 && hitRate <= 1.0, + "CacheHitRate must be in (0.0, 1.0] but was: " + hitRate); + double l1HitRate = catalog.getL1CacheHitRate(); + Assertions.assertTrue(l1HitRate > 0.0 && l1HitRate <= 1.0, + "L1CacheHitRate must be in (0.0, 1.0] but was: " + l1HitRate); + } + + /** + * Verifies that the {@code resetCacheStats} JMX operation zeroes all counters, and that the + * getters and JMX attributes report the same values. + * + *

Strategy: + *

    + *
  1. Perform some cache operations to ensure counters are non-zero.
  2. + *
  3. Invoke {@code resetCacheStats()} via JMX.
  4. + *
  5. Assert that every JMX counter attribute reads {@code 0} / {@code 0.0}.
  6. + *
  7. Drive further loads and confirm the counters resume from zero.
  8. + *
+ */ + @Test + void testJmxResetCacheStats() throws Exception { + Table created = hiveCatalog.createTable(TABLE_ID, new Schema()); + Table loaded = catalog.loadTable(TABLE_ID); // miss + load + Assertions.assertEquals(created.location(), loaded.location(), + "Warm-up load must return the table we just created"); + catalog.loadTable(TABLE_ID); // hit (L1 hit on the fast path) + + // Sanity: at least one counter must be non-zero before the reset. + Assertions.assertTrue(jmxLong("CacheHitCount") + jmxLong("CacheMissCount") > 0, + "At least one counter must be non-zero before reset"); + + // -- invoke the reset operation via JMX ------------------------------------- + invokeJmxOperation("resetCacheStats"); + + // -- assertions post-reset -------------------------------------------------- + Assertions.assertEquals(0L, jmxLong("CacheHitCount"), "CacheHitCount must be 0 after reset"); + Assertions.assertEquals(0L, jmxLong("CacheMissCount"), "CacheMissCount must be 0 after reset"); + Assertions.assertEquals(0L, jmxLong("CacheLoadCount"), "CacheLoadCount must be 0 after reset"); + Assertions.assertEquals(0L, jmxLong("CacheInvalidateCount"), "CacheInvalidateCount must be 0 after reset"); + Assertions.assertEquals(0L, jmxLong("CacheMetaLoadCount"), "CacheMetaLoadCount must be 0 after reset"); + Assertions.assertEquals(0L, jmxLong("L1CacheHitCount"), "L1CacheHitCount must be 0 after reset"); + Assertions.assertEquals(0L, jmxLong("L1CacheMissCount"), "L1CacheMissCount must be 0 after reset"); + Assertions.assertEquals(0.0, jmxDouble("CacheHitRate"), 1e-9, "CacheHitRate must be 0.0 after reset"); + Assertions.assertEquals(0.0, jmxDouble("L1CacheHitRate"), 1e-9, "L1CacheHitRate must be 0.0 after reset"); + + // -- verify rate calculation still works correctly after reset -------------- + // resetCacheStats() zeroes counters but does NOT evict the L2/L1 cache, so the table is still + // cached. Invalidate it so the first post-reset load is a genuine cold miss rather than a hit. + catalog.invalidateTable(TABLE_ID); + + // First load after reset: cache miss + load (L1 cold, L2 cold). + catalog.loadTable(TABLE_ID); + // Second and third loads: L1 hits (within TTL). + catalog.loadTable(TABLE_ID); + catalog.loadTable(TABLE_ID); + + // CacheHitRate: 2 hits out of 3 total accesses → ≈ 0.667 + double hitRateAfterReset = jmxDouble("CacheHitRate"); + Assertions.assertTrue(hitRateAfterReset > 0.0 && hitRateAfterReset <= 1.0, + "CacheHitRate must be in (0.0, 1.0] after post-reset operations, but was: " + hitRateAfterReset); + + // Underlying counters must reflect the just-performed operations. + Assertions.assertTrue(jmxLong("CacheHitCount") >= 2, + "CacheHitCount must be >= 2 after two rapid re-loads post-reset"); + Assertions.assertTrue(jmxLong("CacheMissCount") >= 1, + "CacheMissCount must be >= 1 after the first cold load post-reset"); + + // L1CacheHitRate: the 2nd and 3rd loads should have been served by L1. + double l1HitRateAfterReset = jmxDouble("L1CacheHitRate"); + Assertions.assertTrue(l1HitRateAfterReset > 0.0 && l1HitRateAfterReset <= 1.0, + "L1CacheHitRate must be in (0.0, 1.0] after post-reset L1 hits, but was: " + l1HitRateAfterReset); + Assertions.assertTrue(jmxLong("L1CacheHitCount") >= 2, + "L1CacheHitCount must be >= 2 after two rapid re-loads within TTL post-reset"); + } +} diff --git a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestIcebergAuthorizer.java b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestIcebergAuthorizer.java index 0d13414a0074..b10bcca6d8f0 100644 --- a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestIcebergAuthorizer.java +++ b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/TestIcebergAuthorizer.java @@ -9,11 +9,12 @@ * * http://www.apache.org/licenses/LICENSE-2.0 * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. */ package org.apache.iceberg.rest; @@ -32,6 +33,7 @@ import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hive.metastore.api.PrincipalType; import org.apache.hadoop.hive.metastore.conf.MetastoreConf; @@ -45,6 +47,7 @@ import org.apache.hadoop.security.UserGroupInformation; import org.apache.iceberg.Schema; import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.ForbiddenException; import org.apache.iceberg.rest.extension.MockHiveAuthorizer; import org.apache.iceberg.rest.extension.MockHiveAuthorizerFactory; @@ -252,6 +255,143 @@ void testValidateStageCreateTableRejected() throws Exception { Assertions.assertSame(failure, exception.getCause()); } + @Test + @SuppressWarnings("unchecked") + void testAuthorizeLoadTable() throws Exception { + var hiveAuthorizer = mock(HiveAuthorizer.class); + var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer); + + icebergAuthorizer.authorizeLoadTable(CATALOG_NAME, TableIdentifier.of(NAMESPACE, TABLE_NAME)); + + var operation = ArgumentCaptor.forClass(HiveOperationType.class); + var inputs = ArgumentCaptor.forClass(List.class); + var outputs = ArgumentCaptor.forClass(List.class); + verify(hiveAuthorizer).checkPrivileges(operation.capture(), inputs.capture(), outputs.capture(), any()); + + Assertions.assertEquals(HiveOperationType.QUERY, operation.getValue()); + Assertions.assertEquals(List.of(), outputs.getValue()); + Assertions.assertEquals(1, inputs.getValue().size()); + var input = (HivePrivilegeObject) inputs.getValue().getFirst(); + assertThat(input.getType()).isEqualTo(HivePrivilegeObjectType.TABLE_OR_VIEW); + assertThat(input.getCatName()).isEqualTo(CATALOG_NAME); + assertThat(input.getDbname()).isEqualTo(NAMESPACE.level(0)); + assertThat(input.getObjectName()).isEqualTo(TABLE_NAME); + } + + @Test + @SuppressWarnings("unchecked") + void testAuthorizeLoadTableNormalizesMetadataTable() throws Exception { + var hiveAuthorizer = mock(HiveAuthorizer.class); + var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer); + + // A metadata-table identifier db.table.snapshots must be checked against its base table. + var metadataTable = TableIdentifier.of(Namespace.of(NAMESPACE.level(0), TABLE_NAME), "snapshots"); + icebergAuthorizer.authorizeLoadTable(CATALOG_NAME, metadataTable); + + var inputs = ArgumentCaptor.forClass(List.class); + verify(hiveAuthorizer).checkPrivileges(any(), inputs.capture(), anyList(), any()); + var input = (HivePrivilegeObject) inputs.getValue().getFirst(); + assertThat(input.getType()).isEqualTo(HivePrivilegeObjectType.TABLE_OR_VIEW); + assertThat(input.getDbname()).isEqualTo(NAMESPACE.level(0)); + assertThat(input.getObjectName()).isEqualTo(TABLE_NAME); + } + + @Test + @SuppressWarnings("unchecked") + void testFilterTables() throws Exception { + var hiveAuthorizer = mock(HiveAuthorizer.class); + var visible = TableIdentifier.of(NAMESPACE, "visible"); + var hidden = TableIdentifier.of(NAMESPACE, "hidden"); + // Only "visible" survives the filter. + Mockito.when(hiveAuthorizer.filterListCmdObjects(anyList(), any())).thenAnswer(invocation -> { + List objects = invocation.getArgument(0); + return objects.stream().filter(o -> "visible".equals(o.getObjectName())).collect(Collectors.toList()); + }); + var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer); + + Assertions.assertEquals(List.of(visible), icebergAuthorizer.filterTables(CATALOG_NAME, List.of(visible, hidden))); + + var objects = ArgumentCaptor.forClass(List.class); + verify(hiveAuthorizer).filterListCmdObjects(objects.capture(), any()); + var passed = (HivePrivilegeObject) objects.getValue().getFirst(); + assertThat(passed.getType()).isEqualTo(HivePrivilegeObjectType.TABLE_OR_VIEW); + assertThat(passed.getCatName()).isEqualTo(CATALOG_NAME); + assertThat(passed.getDbname()).isEqualTo(NAMESPACE.level(0)); + } + + @Test + void testFilterViews() throws Exception { + var hiveAuthorizer = mock(HiveAuthorizer.class); + var visible = TableIdentifier.of(NAMESPACE, "visible_view"); + var hidden = TableIdentifier.of(NAMESPACE, "hidden_view"); + Mockito.when(hiveAuthorizer.filterListCmdObjects(anyList(), any())).thenAnswer(invocation -> { + List objects = invocation.getArgument(0); + return objects.stream().filter(o -> "visible_view".equals(o.getObjectName())).collect(Collectors.toList()); + }); + var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer); + + Assertions.assertEquals(List.of(visible), icebergAuthorizer.filterViews(CATALOG_NAME, List.of(visible, hidden))); + } + + @Test + void testFilterNamespaces() throws Exception { + var hiveAuthorizer = mock(HiveAuthorizer.class); + var visible = Namespace.of("visible_db"); + var hidden = Namespace.of("hidden_db"); + Mockito.when(hiveAuthorizer.filterListCmdObjects(anyList(), any())).thenAnswer(invocation -> { + List objects = invocation.getArgument(0); + return objects.stream().filter(o -> "visible_db".equals(o.getDbname())).collect(Collectors.toList()); + }); + var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer); + + Assertions.assertEquals( + List.of(visible), icebergAuthorizer.filterNamespaces(CATALOG_NAME, List.of(visible, hidden))); + } + + @Test + void testFilterNamespacesSkipsMultiLevel() throws Exception { + var hiveAuthorizer = mock(HiveAuthorizer.class); + Mockito.when(hiveAuthorizer.filterListCmdObjects(anyList(), any())).thenAnswer(invocation -> invocation.getArgument(0)); + var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer); + + var single = Namespace.of("db"); + var multi = Namespace.of("db", "nested"); + // Multi-level namespaces cannot map to a Hive database and are dropped rather than failing. + Assertions.assertEquals( + List.of(single), icebergAuthorizer.filterNamespaces(CATALOG_NAME, List.of(single, multi))); + } + + @Test + void testFilterWithoutAuthorizer() { + var icebergAuthorizer = new IcebergAuthorizer(() -> null); + var tables = List.of(TableIdentifier.of(NAMESPACE, TABLE_NAME)); + var namespaces = List.of(NAMESPACE); + // Permissive when no authorizer is configured: the full listing is returned unchanged. + Assertions.assertEquals(tables, icebergAuthorizer.filterTables(CATALOG_NAME, tables)); + Assertions.assertEquals(tables, icebergAuthorizer.filterViews(CATALOG_NAME, tables)); + Assertions.assertEquals(namespaces, icebergAuthorizer.filterNamespaces(CATALOG_NAME, namespaces)); + } + + @Test + void testAuthorizeLoadTableRejected() throws Exception { + var hiveAuthorizer = mock(HiveAuthorizer.class); + var failure = new HiveAccessControlException("access denied"); + doThrow(failure).when(hiveAuthorizer).checkPrivileges(any(), anyList(), anyList(), any()); + var icebergAuthorizer = new IcebergAuthorizer(() -> hiveAuthorizer); + + var exception = Assertions.assertThrows(ForbiddenException.class, () -> + icebergAuthorizer.authorizeLoadTable(CATALOG_NAME, TableIdentifier.of(NAMESPACE, TABLE_NAME))); + Assertions.assertEquals("access denied", exception.getMessage()); + Assertions.assertSame(failure, exception.getCause()); + } + + @Test + void testAuthorizeReadWithoutAuthorizer() { + var icebergAuthorizer = new IcebergAuthorizer(() -> null); + // Permissive when no authorizer is configured. + icebergAuthorizer.authorizeLoadTable(CATALOG_NAME, TableIdentifier.of(NAMESPACE, TABLE_NAME)); + } + @Test void testTranslateAuthorizationPluginException() throws Exception { HiveAuthorizer hiveAuthorizer = mock(HiveAuthorizer.class); @@ -262,7 +402,7 @@ void testTranslateAuthorizationPluginException() throws Exception { var request = stageCreateRequest(LOCATION, null); var exception = Assertions.assertThrows(IllegalStateException.class, () -> icebergAuthorizer.validateStageCreateTable(CATALOG_NAME, NAMESPACE, Map.of(), request)); - Assertions.assertEquals("Failed to check privileges stage-create", exception.getMessage()); + Assertions.assertEquals("Failed to check privileges for CREATETABLE", exception.getMessage()); Assertions.assertSame(failure, exception.getCause()); } } diff --git a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/HiveRESTCatalogServerExtension.java b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/HiveRESTCatalogServerExtension.java index 03374c6072ff..e0fd0fd1885c 100644 --- a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/HiveRESTCatalogServerExtension.java +++ b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/HiveRESTCatalogServerExtension.java @@ -31,6 +31,9 @@ import org.apache.hadoop.hive.metastore.ServletSecurity.AuthType; import org.apache.hadoop.hive.metastore.conf.MetastoreConf; import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars; +import org.apache.iceberg.hive.HiveCatalog; +import org.apache.iceberg.rest.HMSCachingCatalog; +import org.apache.iceberg.rest.HMSCatalogFactory; import org.junit.jupiter.api.extension.AfterAllCallback; import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.BeforeEachCallback; @@ -130,6 +133,32 @@ public String getRestEndpoint() { return restCatalogServer.getRestEndpoint(); } + /** + * Builds a fresh {@link HiveCatalog} bound to the embedded metastore through the same production + * path the server uses ({@link HMSCatalogFactory#createHiveCatalog}). The Thrift URI is taken from + * the port the metastore actually bound to, so callers get a working client regardless of how the + * server's own configuration was mutated at startup. + * + * @return a newly initialized HiveCatalog + */ + public HiveCatalog newServerCatalog() { + Configuration catalogConf = new Configuration(conf); + MetastoreConf.setVar(catalogConf, ConfVars.THRIFT_URIS, restCatalogServer.getThriftUri()); + return HMSCatalogFactory.createHiveCatalog(catalogConf); + } + + /** + * Wraps a fresh {@link #newServerCatalog()} in an {@link HMSCachingCatalog} with the given L2 + * expiry, so tests can exercise the caching catalog directly without reaching into the server's + * own instance. + * + * @param expiryMs the L2 cache expiry in milliseconds + * @return a newly created caching catalog + */ + public HMSCachingCatalog newCachingCatalog(long expiryMs) { + return new HMSCachingCatalog(newServerCatalog(), expiryMs); + } + public String getOAuth2TokenEndpoint() { return authorizationServer.getTokenEndpoint(); } diff --git a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/MockHiveAuthorizer.java b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/MockHiveAuthorizer.java index 86b1cde870e3..5b84b16c0a5c 100644 --- a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/MockHiveAuthorizer.java +++ b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/MockHiveAuthorizer.java @@ -138,7 +138,12 @@ private boolean containsDeniedUri(HivePrivilegeObject priv) { @Override public List filterListCmdObjects(List listObjs, HiveAuthzContext context) { - return List.of(); + // Mirror checkPrivileges: the fully-denied user sees nothing, while read-only and regular users + // are allowed to read and therefore see the whole listing. + if (PERMISSION_TEST_USER.equals(authenticator.getUserName())) { + return List.of(); + } + return listObjs; } @Override diff --git a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/RESTCatalogServer.java b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/RESTCatalogServer.java index f04f5c22ccc7..1512e5e834b1 100644 --- a/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/RESTCatalogServer.java +++ b/standalone-metastore/metastore-rest-catalog/src/test/java/org/apache/iceberg/rest/extension/RESTCatalogServer.java @@ -108,4 +108,9 @@ public Path getWarehouseDir() { public String getRestEndpoint() { return String.format("http://localhost:%d/iceberg", restPort); } + + /** Thrift URI of the embedded metastore, built from the port it actually bound to. */ + public String getThriftUri() { + return String.format("thrift://localhost:%d", hmsPort); + } }