diff --git a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java index 5e44069bbe61..575adf0e1d60 100644 --- a/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java +++ b/standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java @@ -1870,10 +1870,6 @@ public enum ConfVars { "hive.metastore.iceberg.catalog.servlet.auth", "jwt", new StringSetValidator("simple", "jwt"), "HMS Iceberg Catalog servlet authentication method (simple or jwt)." ), - ICEBERG_CATALOG_CACHE_EXPIRY("metastore.iceberg.catalog.cache.expiry", - "hive.metastore.iceberg.catalog.cache.expiry", 60_000L, - "HMS Iceberg Catalog cache expiry." - ), HTTPSERVER_THREADPOOL_MIN("hive.metastore.httpserver.threadpool.min", "hive.metastore.httpserver.threadpool.min", 8, "HMS embedded HTTP server minimum number of threads." 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 deleted file mode 100644 index 6b5d76f818a8..000000000000 --- a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCachingCatalog.java +++ /dev/null @@ -1,85 +0,0 @@ -/* - * 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 com.github.benmanes.caffeine.cache.Ticker; -import java.util.List; -import java.util.Map; -import java.util.Set; -import org.apache.iceberg.CachingCatalog; -import org.apache.iceberg.catalog.Catalog; -import org.apache.iceberg.catalog.Namespace; -import org.apache.iceberg.catalog.SupportsNamespaces; -import org.apache.iceberg.catalog.TableIdentifier; -import org.apache.iceberg.exceptions.NamespaceNotEmptyException; -import org.apache.iceberg.exceptions.NoSuchNamespaceException; - - -/** - * Class that wraps an Iceberg Catalog to cache tables. - * @param the catalog class - */ -public class HMSCachingCatalog extends CachingCatalog implements SupportsNamespaces { - protected final CATALOG nsCatalog; - - public HMSCachingCatalog(CATALOG catalog, long expiration) { - super(catalog, true, expiration, Ticker.systemTicker()); - nsCatalog = catalog; - } - - public CATALOG hmsUnwrap() { - return nsCatalog; - } - - @Override - public void createNamespace(Namespace nmspc, Map map) { - nsCatalog.createNamespace(nmspc, map); - } - - @Override - public List listNamespaces(Namespace nmspc) throws NoSuchNamespaceException { - return nsCatalog.listNamespaces(nmspc); - } - - @Override - public Map loadNamespaceMetadata(Namespace nmspc) throws NoSuchNamespaceException { - return nsCatalog.loadNamespaceMetadata(nmspc); - } - - @Override - public boolean dropNamespace(Namespace nmspc) throws NamespaceNotEmptyException { - List tables = listTables(nmspc); - for (TableIdentifier ident : tables) { - invalidateTable(ident); - } - return nsCatalog.dropNamespace(nmspc); - } - - @Override - public boolean setProperties(Namespace nmspc, Map map) throws NoSuchNamespaceException { - return nsCatalog.setProperties(nmspc, map); - } - - @Override - public boolean removeProperties(Namespace nmspc, Set set) throws NoSuchNamespaceException { - return nsCatalog.removeProperties(nmspc, set); - } - -} 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 dbf280396f1e..699661612d49 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 @@ -46,10 +46,12 @@ import org.apache.iceberg.exceptions.NoSuchIcebergTableException; import org.apache.iceberg.exceptions.NoSuchNamespaceException; import org.apache.iceberg.exceptions.NoSuchTableException; +import org.apache.iceberg.exceptions.NoSuchViewException; import org.apache.iceberg.exceptions.NotAuthorizedException; import org.apache.iceberg.exceptions.RESTException; import org.apache.iceberg.exceptions.UnprocessableEntityException; import org.apache.iceberg.exceptions.ValidationException; +import org.apache.iceberg.hive.HiveCatalog; import org.apache.iceberg.relocated.com.google.common.base.Splitter; import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap; import org.apache.iceberg.relocated.com.google.common.collect.Lists; @@ -86,14 +88,14 @@ public class HMSCatalogAdapter implements RESTClient { private static final Map, Integer> EXCEPTION_ERROR_CODES = ImmutableMap., Integer>builder() - .put(NamespaceNotSupported.class, 400) .put(IllegalArgumentException.class, 400) .put(ValidationException.class, 400) - .put(NamespaceNotEmptyException.class, 400) + .put(NamespaceNotEmptyException.class, 409) .put(NotAuthorizedException.class, 401) .put(ForbiddenException.class, 403) .put(NoSuchNamespaceException.class, 404) .put(NoSuchTableException.class, 404) + .put(NoSuchViewException.class, 404) .put(NoSuchIcebergTableException.class, 404) .put(UnsupportedOperationException.class, 406) .put(AlreadyExistsException.class, 409) @@ -118,11 +120,10 @@ public class HMSCatalogAdapter implements RESTClient { private final ViewCatalog asViewCatalog; - public HMSCatalogAdapter(Catalog catalog) { + public HMSCatalogAdapter(HiveCatalog catalog) { this.catalog = catalog; - this.asNamespaceCatalog = - catalog instanceof SupportsNamespaces ? (SupportsNamespaces) catalog : null; - this.asViewCatalog = catalog instanceof ViewCatalog ? (ViewCatalog) catalog : null; + this.asNamespaceCatalog = catalog; + this.asViewCatalog = catalog; } enum HTTPMethod { @@ -326,53 +327,36 @@ private OAuthTokenResponse tokens(Object body) { } private ListNamespacesResponse listNamespaces(Map vars) { - if (asNamespaceCatalog != null) { - Namespace namespace; - if (vars.containsKey("parent")) { - namespace = Namespace.of(RESTUtil.NAMESPACE_SPLITTER.splitToStream(vars.get("parent")).toArray(String[]::new)); - } else { - namespace = Namespace.empty(); - } - return castResponse(ListNamespacesResponse.class, CatalogHandlers.listNamespaces(asNamespaceCatalog, namespace)); + Namespace namespace; + if (vars.containsKey("parent")) { + namespace = Namespace.of(RESTUtil.NAMESPACE_SPLITTER.splitToStream(vars.get("parent")).toArray(String[]::new)); + } else { + namespace = Namespace.empty(); } - throw new NamespaceNotSupported(catalog.toString()); + return castResponse(ListNamespacesResponse.class, CatalogHandlers.listNamespaces(asNamespaceCatalog, namespace)); } private CreateNamespaceResponse createNamespace(Object body) { - if (asNamespaceCatalog != null) { - CreateNamespaceRequest request = castRequest(CreateNamespaceRequest.class, body); - return castResponse( - CreateNamespaceResponse.class, CatalogHandlers.createNamespace(asNamespaceCatalog, request)); - } - throw new NamespaceNotSupported(catalog.toString()); + CreateNamespaceRequest request = castRequest(CreateNamespaceRequest.class, body); + return castResponse( + CreateNamespaceResponse.class, CatalogHandlers.createNamespace(asNamespaceCatalog, request)); } private GetNamespaceResponse loadNamespace(Map vars) { - if (asNamespaceCatalog != null) { - Namespace namespace = namespaceFromPathVars(vars); - return castResponse( - GetNamespaceResponse.class, CatalogHandlers.loadNamespace(asNamespaceCatalog, namespace)); - } - throw new NamespaceNotSupported(catalog.toString()); + Namespace namespace = namespaceFromPathVars(vars); + return castResponse(GetNamespaceResponse.class, CatalogHandlers.loadNamespace(asNamespaceCatalog, namespace)); } private RESTResponse dropNamespace(Map vars) { - if (asNamespaceCatalog != null) { - CatalogHandlers.dropNamespace(asNamespaceCatalog, namespaceFromPathVars(vars)); - } - throw new NamespaceNotSupported(catalog.toString()); + CatalogHandlers.dropNamespace(asNamespaceCatalog, namespaceFromPathVars(vars)); + return null; } private UpdateNamespacePropertiesResponse updateNamespace(Map vars, Object body) { - if (asNamespaceCatalog != null) { - Namespace namespace = namespaceFromPathVars(vars); - UpdateNamespacePropertiesRequest request = - castRequest(UpdateNamespacePropertiesRequest.class, body); - return castResponse( - UpdateNamespacePropertiesResponse.class, - CatalogHandlers.updateNamespaceProperties(asNamespaceCatalog, namespace, request)); - } - throw new NamespaceNotSupported(catalog.toString()); + Namespace namespace = namespaceFromPathVars(vars); + UpdateNamespacePropertiesRequest request = castRequest(UpdateNamespacePropertiesRequest.class, body); + return castResponse(UpdateNamespacePropertiesResponse.class, + CatalogHandlers.updateNamespaceProperties(asNamespaceCatalog, namespace, request)); } private ListTablesResponse listTables(Map vars) { @@ -386,11 +370,9 @@ private LoadTableResponse createTable(Map vars, Object body) { CreateTableRequest request = castRequest(CreateTableRequest.class, body); request.validate(); if (request.stageCreate()) { - return castResponse( - responseType, CatalogHandlers.stageTableCreate(catalog, namespace, request)); + return castResponse(responseType, CatalogHandlers.stageTableCreate(catalog, namespace, request)); } else { - return castResponse( - responseType, CatalogHandlers.createTable(catalog, namespace, request)); + return castResponse(responseType, CatalogHandlers.createTable(catalog, namespace, request)); } } @@ -439,65 +421,43 @@ private RESTResponse commitTransaction(Object body) { } private ListTablesResponse listViews(Map vars) { - if (null != asViewCatalog) { - 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)); - } + 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)); } - throw new ViewNotSupported(catalog.toString()); } private LoadViewResponse createView(Map vars, Object body) { - if (null != asViewCatalog) { - Namespace namespace = namespaceFromPathVars(vars); - CreateViewRequest request = castRequest(CreateViewRequest.class, body); - return castResponse( - LoadViewResponse.class, CatalogHandlers.createView(asViewCatalog, namespace, request)); - } - throw new ViewNotSupported(catalog.toString()); + Namespace namespace = namespaceFromPathVars(vars); + CreateViewRequest request = castRequest(CreateViewRequest.class, body); + return castResponse(LoadViewResponse.class, CatalogHandlers.createView(asViewCatalog, namespace, request)); } private LoadViewResponse loadView(Map vars) { - if (null != asViewCatalog) { - TableIdentifier ident = identFromPathVars(vars); - return castResponse(LoadViewResponse.class, CatalogHandlers.loadView(asViewCatalog, ident)); - } - throw new ViewNotSupported(catalog.toString()); + TableIdentifier ident = identFromPathVars(vars); + return castResponse(LoadViewResponse.class, CatalogHandlers.loadView(asViewCatalog, ident)); } private LoadViewResponse updateView(Map vars, Object body) { - if (null != asViewCatalog) { - TableIdentifier ident = identFromPathVars(vars); - UpdateTableRequest request = castRequest(UpdateTableRequest.class, body); - return castResponse( - LoadViewResponse.class, CatalogHandlers.updateView(asViewCatalog, ident, request)); - } - throw new ViewNotSupported(catalog.toString()); + TableIdentifier ident = identFromPathVars(vars); + UpdateTableRequest request = castRequest(UpdateTableRequest.class, body); + return castResponse(LoadViewResponse.class, CatalogHandlers.updateView(asViewCatalog, ident, request)); } private RESTResponse renameView(Object body) { - if (null != asViewCatalog) { - RenameTableRequest request = castRequest(RenameTableRequest.class, body); - CatalogHandlers.renameView(asViewCatalog, request); + RenameTableRequest request = castRequest(RenameTableRequest.class, body); + CatalogHandlers.renameView(asViewCatalog, request); return null; - } - throw new ViewNotSupported(catalog.toString()); } private RESTResponse dropView(Map vars) { - if (null != asViewCatalog) { - CatalogHandlers.dropView(asViewCatalog, identFromPathVars(vars)); - return null; - } - throw new ViewNotSupported(catalog.toString()); + CatalogHandlers.dropView(asViewCatalog, identFromPathVars(vars)); + return null; } /** @@ -703,18 +663,6 @@ public void close() { // The caller is responsible for closing the underlying catalog backing this REST catalog. } - private static class NamespaceNotSupported extends RuntimeException { - NamespaceNotSupported(String catalog) { - super("catalog " + catalog + " does not support namespace"); - } - } - - private static class ViewNotSupported extends RuntimeException { - ViewNotSupported(String catalog) { - super("catalog " + catalog + " does not support views"); - } - } - private static class BadResponseType extends RuntimeException { private BadResponseType(Class responseType, Object response) { super( 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 51a03a42b796..3aa9f3458890 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 @@ -18,7 +18,6 @@ */ package org.apache.iceberg.rest; -import java.io.IOException; import java.lang.ref.Reference; import java.lang.ref.SoftReference; import java.util.Map; @@ -30,47 +29,41 @@ import org.apache.hadoop.hive.metastore.ServletServerBuilder; import org.apache.hadoop.hive.metastore.conf.MetastoreConf; import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars; -import org.apache.iceberg.catalog.Catalog; import org.apache.iceberg.hive.HiveCatalog; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Catalog & servlet factory. */ public class HMSCatalogFactory { - private static final Logger LOG = LoggerFactory.getLogger(HMSCatalogFactory.class); /** * Convenience soft reference to last catalog. */ - protected static final AtomicReference> catalogRef = new AtomicReference<>(); + private static final AtomicReference> catalogRef = new AtomicReference<>(); - public static Catalog getLastCatalog() { - Reference soft = catalogRef.get(); + public static HiveCatalog getLastCatalog() { + Reference soft = catalogRef.get(); return soft != null ? soft.get() : null; } - protected static void setLastCatalog(Catalog catalog) { + private static void setLastCatalog(HiveCatalog catalog) { catalogRef.set(new SoftReference<>(catalog)); } - protected final Configuration configuration; - protected final int port; - protected final String path; - protected Catalog catalog; + private final Configuration configuration; + private final int port; + private final String path; + private HiveCatalog catalog; /** * Factory constructor. *

Called by the static method {@link HMSCatalogFactory#createServlet(Configuration)} that is * declared in configuration and found through introspection.

* @param conf the configuration - * @param catalog the catalog */ - protected HMSCatalogFactory(Configuration conf, Catalog catalog) { + private HMSCatalogFactory(Configuration conf) { port = MetastoreConf.getIntVar(conf, MetastoreConf.ConfVars.ICEBERG_CATALOG_SERVLET_PORT); path = MetastoreConf.getVar(conf, MetastoreConf.ConfVars.ICEBERG_CATALOG_SERVLET_PATH); this.configuration = conf; - this.catalog = catalog; } public int getPort() { @@ -80,16 +73,12 @@ public int getPort() { public String getPath() { return path; } - - public Catalog getCatalog() { - return catalog; - } /** * Creates the catalog instance. * @return the catalog */ - protected Catalog createCatalog() { + private HiveCatalog createCatalog() { final Map properties = new TreeMap<>(); MetastoreConf.setVar(configuration, MetastoreConf.ConfVars.THRIFT_URIS, ""); final String configUri = MetastoreConf.getVar(configuration, MetastoreConf.ConfVars.THRIFT_URIS); @@ -108,8 +97,7 @@ protected 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; } /** @@ -117,7 +105,7 @@ protected Catalog createCatalog() { * @param catalog the Iceberg catalog * @return the servlet */ - protected HttpServlet createServlet(Catalog catalog) { + private HttpServlet createServlet(HiveCatalog catalog) { String authType = MetastoreConf.getVar(configuration, ConfVars.ICEBERG_CATALOG_SERVLET_AUTH); ServletSecurity security = new ServletSecurity(authType, configuration); return security.proxy(new HMSCatalogServlet(new HMSCatalogAdapter(catalog))); @@ -126,11 +114,10 @@ protected HttpServlet createServlet(Catalog catalog) { /** * Creates the REST catalog servlet instance. * @return the servlet - * @throws IOException if creation fails */ - protected HttpServlet createServlet() throws IOException { + private HttpServlet createServlet() { if (port >= 0 && path != null && !path.isEmpty()) { - Catalog actualCatalog = catalog; + HiveCatalog actualCatalog = catalog; if (actualCatalog == null) { actualCatalog = catalog = createCatalog(); } @@ -150,15 +137,11 @@ protected HttpServlet createServlet() throws IOException { */ @SuppressWarnings("unused") public static ServletServerBuilder.Descriptor createServlet(Configuration configuration) { - try { - HMSCatalogFactory hms = new HMSCatalogFactory(configuration, null); - HttpServlet servlet = hms.createServlet(); - if (servlet != null) { - return new ServletServerBuilder.Descriptor(hms.getPort(), hms.getPath(), servlet); - } - } catch (IOException exception) { - LOG.error("failed to create servlet ", exception); + HMSCatalogFactory hms = new HMSCatalogFactory(configuration); + HttpServlet servlet = hms.createServlet(); + if (servlet == null) { + return null; } - return null; + return new ServletServerBuilder.Descriptor(hms.getPort(), hms.getPath(), servlet); } }