henrib commented on code in PR #6441: URL: https://github.com/apache/hive/pull/6441#discussion_r3791964789
########## standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/hive/MetadataLocator.java: ########## @@ -0,0 +1,108 @@ +/* + * 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.iceberg.exceptions.NoSuchTableException; +import org.apache.thrift.TException; + +/** + * Fetches the location of a given metadata table. + * <p>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.</p> + */ +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 location of the metadata table identified by the given identifier, or null if the table is + * not a metadata table. + * <p>This uses the Thrift API to fetch the table parameters, which is more efficient than fetching the entire table object.</p> + * @param identifier the identifier of the metadata table to fetch the location for + * @return the location of the metadata table, or null if the table does not exist or is not a metadata table + * @throws NoSuchTableException if the table does not exist + */ + public String getLocation(TableIdentifier identifier) { + final ClientPool<IMetaStoreClient, TException> 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<Table> 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) { Review Comment: Adopted — switched to validateIcebergViewNotLoadedAsIcebergTable(table, baseTableIdentifier.toString()). ########## standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java: ########## @@ -80,6 +80,11 @@ 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()); Review Comment: The RESTException branch stays: CatalogHandlers throws REST exceptions that are already mapped to a response, so we log at DEBUG to avoid double-handling. Happy to drop it if you prefer letting it fall through. ########## standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCachingCatalog.java: ########## @@ -33,65 +51,423 @@ import org.apache.iceberg.exceptions.NamespaceNotEmptyException; import org.apache.iceberg.exceptions.NoSuchNamespaceException; 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; +import com.github.benmanes.caffeine.cache.Ticker; /** * Class that wraps an Iceberg Catalog to cache tables. */ -public class HMSCachingCatalog extends CachingCatalog implements SupportsNamespaces, ViewCatalog { +public class HMSCachingCatalog extends CachingCatalog + implements SupportsNamespaces, ViewCatalog, HMSCachingCatalogMXBean, Closeable { + protected static final Logger LOG = LoggerFactory.getLogger(HMSCachingCatalog.class); + + @TestOnly + private static SoftReference<HMSCachingCatalog> cacheRef = new SoftReference<>(null); + + @TestOnly + @SuppressWarnings("unchecked") + public static <C extends Catalog> C getLatestCache(Function<HMSCachingCatalog, C> extractor) { + HMSCachingCatalog cache = cacheRef.get(); + if (cache == null) { + return null; + } + return extractor == null ? (C) cache : extractor.apply(cache); + } + + @TestOnly + public HiveCatalog getCatalog() { + return hiveCatalog; + } + + // The underlying HiveCatalog instance. private final HiveCatalog hiveCatalog; - - public HMSCachingCatalog(HiveCatalog catalog, long expiration) { - super(catalog, true, expiration, Ticker.systemTicker()); + // Duplicate because CachingCatalog doesn't expose the case sensitivity of the underlying catalog, + // which is needed for canonicalizing identifiers before caching. + private final boolean caseSensitive; + // The locator. + private final MetadataLocator metadataLocator; + // 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<TableIdentifier, Long> l1Cache; + // The TTL for L1 cache (3s). + private final int l1Ttl; + // The L1 cache size. + private final int l1CacheSize; + + // Metrics counters. + private final AtomicLong cacheHitCount = new AtomicLong(0); + private final AtomicLong cacheMissCount = new AtomicLong(0); + private final AtomicLong cacheLoadCount = new AtomicLong(0); + private final AtomicLong cacheInvalidateCount = new AtomicLong(0); + private final AtomicLong cacheMetaLoadCount = new AtomicLong(0); + // L1 cache metrics: counted only when the L2 (Caffeine) cache already has the entry. + private final AtomicLong l1CacheHitCount = new AtomicLong(0); + private final AtomicLong l1CacheMissCount = new AtomicLong(0); + + // JMX ObjectName under which this instance is registered (may be null if registration failed). + private ObjectName jmxObjectName; + + public HMSCachingCatalog(HiveCatalog catalog, long expirationMs) { + this(catalog, expirationMs, /*caseSensitive*/ true); + } + + public HMSCachingCatalog(HiveCatalog catalog, long expirationMs, boolean caseSensitive) { + super(catalog, caseSensitive, expirationMs, Ticker.systemTicker()); this.hiveCatalog = catalog; + this.caseSensitive = caseSensitive; + this.metadataLocator = new MetadataLocator(catalog); + Configuration conf = catalog.getConf(); + if (HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_IN_TEST)) { + // Only keep a reference to the latest cache for testing purpose, so that tests can manipulate the catalog. + cacheRef = new SoftReference<>(this); + } + 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) { + l1Cache = Collections.synchronizedMap(new LinkedHashMap<TableIdentifier, Long>() { + @Override + protected boolean removeEldestEntry(Map.Entry<TableIdentifier, Long> eldest) { + return size() > l1CacheSize; + } + }); + l1Ttl = l1ttl; + l1CacheSize = l1size; + } else { + l1Cache = Collections.emptyMap(); + l1Ttl = 0; + l1CacheSize = 0; + } + 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.\\\\-]", "_"); Review Comment: Using catalog.name() is deliberate — the JMX bean is keyed by the actual catalog instance name rather than the global default, so multiple catalogs do not collide. ########## standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCachingCatalog.java: ########## @@ -33,65 +51,423 @@ import org.apache.iceberg.exceptions.NamespaceNotEmptyException; import org.apache.iceberg.exceptions.NoSuchNamespaceException; 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; +import com.github.benmanes.caffeine.cache.Ticker; /** * Class that wraps an Iceberg Catalog to cache tables. */ -public class HMSCachingCatalog extends CachingCatalog implements SupportsNamespaces, ViewCatalog { +public class HMSCachingCatalog extends CachingCatalog + implements SupportsNamespaces, ViewCatalog, HMSCachingCatalogMXBean, Closeable { + protected static final Logger LOG = LoggerFactory.getLogger(HMSCachingCatalog.class); + + @TestOnly + private static SoftReference<HMSCachingCatalog> cacheRef = new SoftReference<>(null); + + @TestOnly + @SuppressWarnings("unchecked") + public static <C extends Catalog> C getLatestCache(Function<HMSCachingCatalog, C> extractor) { + HMSCachingCatalog cache = cacheRef.get(); + if (cache == null) { + return null; + } + return extractor == null ? (C) cache : extractor.apply(cache); + } + + @TestOnly + public HiveCatalog getCatalog() { + return hiveCatalog; + } + + // The underlying HiveCatalog instance. private final HiveCatalog hiveCatalog; - - public HMSCachingCatalog(HiveCatalog catalog, long expiration) { - super(catalog, true, expiration, Ticker.systemTicker()); + // Duplicate because CachingCatalog doesn't expose the case sensitivity of the underlying catalog, + // which is needed for canonicalizing identifiers before caching. + private final boolean caseSensitive; + // The locator. + private final MetadataLocator metadataLocator; + // 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<TableIdentifier, Long> l1Cache; + // The TTL for L1 cache (3s). + private final int l1Ttl; + // The L1 cache size. + private final int l1CacheSize; + + // Metrics counters. + private final AtomicLong cacheHitCount = new AtomicLong(0); + private final AtomicLong cacheMissCount = new AtomicLong(0); + private final AtomicLong cacheLoadCount = new AtomicLong(0); + private final AtomicLong cacheInvalidateCount = new AtomicLong(0); + private final AtomicLong cacheMetaLoadCount = new AtomicLong(0); + // L1 cache metrics: counted only when the L2 (Caffeine) cache already has the entry. + private final AtomicLong l1CacheHitCount = new AtomicLong(0); + private final AtomicLong l1CacheMissCount = new AtomicLong(0); + + // JMX ObjectName under which this instance is registered (may be null if registration failed). + private ObjectName jmxObjectName; + + public HMSCachingCatalog(HiveCatalog catalog, long expirationMs) { + this(catalog, expirationMs, /*caseSensitive*/ true); + } + + public HMSCachingCatalog(HiveCatalog catalog, long expirationMs, boolean caseSensitive) { + super(catalog, caseSensitive, expirationMs, Ticker.systemTicker()); this.hiveCatalog = catalog; + this.caseSensitive = caseSensitive; + this.metadataLocator = new MetadataLocator(catalog); + Configuration conf = catalog.getConf(); + if (HiveConf.getBoolVar(conf, HiveConf.ConfVars.HIVE_IN_TEST)) { + // Only keep a reference to the latest cache for testing purpose, so that tests can manipulate the catalog. + cacheRef = new SoftReference<>(this); + } + 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) { + l1Cache = Collections.synchronizedMap(new LinkedHashMap<TableIdentifier, Long>() { + @Override + protected boolean removeEldestEntry(Map.Entry<TableIdentifier, Long> eldest) { + return size() > l1CacheSize; + } + }); + l1Ttl = l1ttl; + l1CacheSize = l1size; + } else { + l1Cache = Collections.emptyMap(); + l1Ttl = 0; + l1CacheSize = 0; + } + 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.warn("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 + */ + protected void onCacheInvalidate(TableIdentifier tid) { + long count = cacheInvalidateCount.incrementAndGet(); + LOG.debug("Cache invalidate {}: {}", tid, count); + } + + /** + * Callback when cache loads a table for a given table identifier. + * + * @param tid the table identifier + */ + protected void onCacheLoad(TableIdentifier tid) { + long count = cacheLoadCount.incrementAndGet(); + LOG.debug("Cache load {}: {}", tid, count); + } + + /** + * Callback when cache hit for a given table identifier. + * + * @param tid the table identifier + */ + protected void onCacheHit(TableIdentifier tid) { + long count = cacheHitCount.incrementAndGet(); + LOG.debug("Cache hit {} : {}", tid, count); + } + + /** + * Callback when cache miss occurs for a given table identifier. + * + * @param tid the table identifier + */ + protected void onCacheMiss(TableIdentifier tid) { + long count = cacheMissCount.incrementAndGet(); + LOG.debug("Cache miss {}: {}", tid, count); + } + + /** + * Callback when cache loads a metadata table for a given table identifier. + * + * @param tid the table identifier + */ + protected void onCacheMetaLoad(TableIdentifier tid) { + long count = cacheMetaLoadCount.incrementAndGet(); + LOG.debug("Cache meta-load {}: {}", tid, count); + } + + /** + * 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 + */ + protected void onL1CacheHit(TableIdentifier tid) { + long count = l1CacheHitCount.incrementAndGet(); + LOG.debug("L1 cache hit {}: {}", tid, count); } + /** + * 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 + */ + protected void onL1CacheMiss(TableIdentifier tid) { + long count = l1CacheMissCount.incrementAndGet(); + LOG.debug("L1 cache miss {}: {}", tid, count); + } + + // Getter methods for accessing metrics @Override - public Catalog.TableBuilder buildTable(TableIdentifier identifier, Schema schema) { - return hiveCatalog.buildTable(identifier, schema); + public long getCacheHitCount() { + return cacheHitCount.get(); } @Override - public void createNamespace(Namespace nmspc, Map<String, String> map) { - hiveCatalog.createNamespace(nmspc, map); + public long getCacheMissCount() { + return cacheMissCount.get(); } @Override - public List<Namespace> listNamespaces(Namespace nmspc) throws NoSuchNamespaceException { - return hiveCatalog.listNamespaces(nmspc); + public long getCacheLoadCount() { + return cacheLoadCount.get(); } @Override - public Map<String, String> loadNamespaceMetadata(Namespace nmspc) throws NoSuchNamespaceException { - return hiveCatalog.loadNamespaceMetadata(nmspc); + public long getCacheInvalidateCount() { + return cacheInvalidateCount.get(); } @Override - public boolean dropNamespace(Namespace nmspc) throws NamespaceNotEmptyException { - List<TableIdentifier> tables = listTables(nmspc); + public long getCacheMetaLoadCount() { + return cacheMetaLoadCount.get(); + } + + @Override + public double getCacheHitRate() { + long hits = cacheHitCount.get(); + long total = hits + cacheMissCount.get(); + return total == 0 ? 0.0 : (double) hits / total; + } + + @Override + public long getL1CacheHitCount() { + return l1CacheHitCount.get(); + } + + @Override + public long getL1CacheMissCount() { + return l1CacheMissCount.get(); + } + + @Override + public double getL1CacheHitRate() { + long hits = l1CacheHitCount.get(); + long total = hits + l1CacheMissCount.get(); + return total == 0 ? 0.0 : (double) hits / total; + } + + @Override + public void resetCacheStats() { + cacheHitCount.set(0); + cacheMissCount.set(0); + cacheLoadCount.set(0); + cacheInvalidateCount.set(0); + cacheMetaLoadCount.set(0); + l1CacheHitCount.set(0); + l1CacheMissCount.set(0); + LOG.debug("Cache stats reset"); + } + + @Override + 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 void createNamespace(Namespace namespace, Map<String, String> map) { + hiveCatalog.createNamespace(namespace, map); + } + + @Override + public List<Namespace> listNamespaces(Namespace namespace) throws NoSuchNamespaceException { + return hiveCatalog.listNamespaces(namespace); + } + + /** + * Canonicalizes the given table identifier based on the case sensitivity of the underlying catalog. + * Copied from CachingCatalog that exposes it as private. + * @param tableIdentifier the table identifier to canonicalize + * @return the canonicalized table identifier + */ + private TableIdentifier canonicalizeIdentifier(TableIdentifier tableIdentifier) { + return this.caseSensitive ? tableIdentifier : tableIdentifier.toLowerCase(); + } + + @Override + public void invalidateTable(TableIdentifier ident) { + super.invalidateTable(ident); + l1Cache.remove(ident); + } + + @Override + public Table loadTable(final TableIdentifier identifier) { + final TableIdentifier canonicalized = canonicalizeIdentifier(identifier); + final Table cachedTable = tableCache.getIfPresent(canonicalized); + 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(canonicalized); + if (lastCached != null) { + if (now - lastCached < l1Ttl) { + LOG.debug("Table {} is in L1 cache, returning cached table", canonicalized); + onL1CacheHit(canonicalized); + onCacheHit(canonicalized); + return cachedTable; Review Comment: L1 stays as pure caching here. The authorization concern is real and is enforced one layer up in HIVE-29817: every loadTable/loadView gets a hard QUERY check at the REST choke point before the cache is consulted, so cached and uncached reads authorize identically. ########## standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/hive/MetadataLocator.java: ########## @@ -0,0 +1,108 @@ +/* + * 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.iceberg.exceptions.NoSuchTableException; +import org.apache.thrift.TException; + +/** + * Fetches the location of a given metadata table. + * <p>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.</p> + */ +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 location of the metadata table identified by the given identifier, or null if the table is + * not a metadata table. + * <p>This uses the Thrift API to fetch the table parameters, which is more efficient than fetching the entire table object.</p> + * @param identifier the identifier of the metadata table to fetch the location for + * @return the location of the metadata table, or null if the table does not exist or is not a metadata table + * @throws NoSuchTableException if the table does not exist + */ + public String getLocation(TableIdentifier identifier) { + final ClientPool<IMetaStoreClient, TException> clients = catalog.clientPool(); + final String catName = catalog.name(); + final TableIdentifier baseTableIdentifier; + if (!catalog.isValidIdentifier(identifier)) { + if (!isValidMetadataIdentifier(identifier)) { + return null; Review Comment: getLocation deliberately returns null for not-found; the caller (loadTable) maps null to NoSuchTableException, so the REST semantics are preserved without the locator throwing. ########## standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/hive/MetadataLocator.java: ########## @@ -0,0 +1,108 @@ +/* + * 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.iceberg.exceptions.NoSuchTableException; +import org.apache.thrift.TException; + +/** + * Fetches the location of a given metadata table. + * <p>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.</p> + */ +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 location of the metadata table identified by the given identifier, or null if the table is + * not a metadata table. + * <p>This uses the Thrift API to fetch the table parameters, which is more efficient than fetching the entire table object.</p> + * @param identifier the identifier of the metadata table to fetch the location for + * @return the location of the metadata table, or null if the table does not exist or is not a metadata table + * @throws NoSuchTableException if the table does not exist + */ + public String getLocation(TableIdentifier identifier) { + final ClientPool<IMetaStoreClient, TException> 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<Table> 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.validateTableIsIceberg(table, tableName); + return table.getParameters().get(BaseMetastoreTableOperations.METADATA_LOCATION_PROP); + } + } + return null; Review Comment: Same as above — null is the not-found signal here; loadTable maps it to NoSuchTableException. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
