From 8332c33e04cd75660bf373236ecd76cf24f978d8 Mon Sep 17 00:00:00 2001 From: David Phillips Date: Wed, 29 Nov 2023 18:58:25 -0800 Subject: [PATCH 1/3] Remove outdated SPI revapi exclusions --- core/trino-spi/pom.xml | 35 ----------------------------------- 1 file changed, 35 deletions(-) diff --git a/core/trino-spi/pom.xml b/core/trino-spi/pom.xml index b4b6e9f9982a..1f76552d4fc1 100644 --- a/core/trino-spi/pom.xml +++ b/core/trino-spi/pom.xml @@ -212,41 +212,6 @@ - - java.class.removed - interface io.trino.spi.block.VariableWidthBlockBuilder.VariableWidthEntryBuilder<E extends java.lang.Throwable> - - - java.method.removed - method <E extends java.lang.Throwable> void io.trino.spi.block.VariableWidthBlockBuilder::buildEntry(io.trino.spi.block.VariableWidthBlockBuilder.VariableWidthEntryBuilder<E>) throws E - - - true - java.method.addedToInterface - method void io.trino.spi.block.BlockBuilder::append(io.trino.spi.block.ValueBlock, int) - - - java.method.addedToInterface - method void io.trino.spi.block.BlockBuilder::appendPositions(io.trino.spi.block.ValueBlock, int[], int, int) - - - java.method.addedToInterface - method void io.trino.spi.block.BlockBuilder::appendRange(io.trino.spi.block.ValueBlock, int, int) - - - java.method.addedToInterface - method void io.trino.spi.block.BlockBuilder::appendRepeated(io.trino.spi.block.ValueBlock, int, int) - - - java.method.returnTypeChangedCovariantly - method io.trino.spi.block.BlockBuilder io.trino.spi.block.ShortArrayBlockBuilder::appendNull() - method io.trino.spi.block.ShortArrayBlockBuilder io.trino.spi.block.ShortArrayBlockBuilder::appendNull() - - - java.method.returnTypeChangedCovariantly - method io.trino.spi.block.BlockBuilder io.trino.spi.block.ShortArrayBlockBuilder::writeShort(short) - method io.trino.spi.block.ShortArrayBlockBuilder io.trino.spi.block.ShortArrayBlockBuilder::writeShort(short) - From 3f8534e07229851b8f583cdae070559f5a139a64 Mon Sep 17 00:00:00 2001 From: David Phillips Date: Wed, 29 Nov 2023 19:14:43 -0800 Subject: [PATCH 2/3] Fix warnings in HandleResolver --- .../main/java/io/trino/metadata/HandleResolver.java | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/metadata/HandleResolver.java b/core/trino-main/src/main/java/io/trino/metadata/HandleResolver.java index 452a568f1783..67690b4b7cd3 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/HandleResolver.java +++ b/core/trino-main/src/main/java/io/trino/metadata/HandleResolver.java @@ -46,11 +46,9 @@ public void unregisterClassLoader(PluginClassLoader classLoader) checkState(result, "Class loader not registered: %s", classLoader.getId()); } - @SuppressWarnings("MethodMayBeStatic") - public String getId(Object tableHandle) + public String getId(Object handle) { - Class handleClass = tableHandle.getClass(); - return classId(handleClass); + return classId(handle.getClass()); } public Class getHandleClass(String id) @@ -80,8 +78,8 @@ private static String classId(Class handleClass) private static String classLoaderId(Class handleClass) { ClassLoader classLoader = handleClass.getClassLoader(); - if (classLoader instanceof PluginClassLoader) { - return ((PluginClassLoader) classLoader).getId(); + if (classLoader instanceof PluginClassLoader pluginClassLoader) { + return pluginClassLoader.getId(); } checkArgument(classLoader == HandleResolver.class.getClassLoader(), "Handle [%s] has unknown class loader [%s]", From f8f686161392c6257e771c8ccc68353a85ee5e0b Mon Sep 17 00:00:00 2001 From: David Phillips Date: Wed, 29 Nov 2023 19:24:25 -0800 Subject: [PATCH 3/3] Remove deprecated duplicatePluginClassLoader --- .../io/trino/connector/CatalogFactory.java | 4 +- .../connector/ConnectorContextInstance.java | 17 +- .../io/trino/connector/ConnectorServices.java | 7 +- .../connector/DefaultCatalogFactory.java | 161 +++--------------- .../trino/connector/LazyCatalogFactory.java | 6 +- .../io/trino/metadata/HandleResolver.java | 6 - .../io/trino/server/PluginClassLoader.java | 26 +-- .../java/io/trino/server/PluginInstaller.java | 5 +- .../java/io/trino/server/PluginManager.java | 12 +- .../server/testing/TestingTrinoServer.java | 2 +- .../io/trino/testing/LocalQueryRunner.java | 11 +- .../testing/TestingConnectorContext.java | 6 - ...estSqlTaskManagerRaceWithCatalogPrune.java | 6 +- .../trino/spi/connector/ConnectorContext.java | 6 - 14 files changed, 47 insertions(+), 228 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/connector/CatalogFactory.java b/core/trino-main/src/main/java/io/trino/connector/CatalogFactory.java index fc1cce6cc606..98af95c81089 100644 --- a/core/trino-main/src/main/java/io/trino/connector/CatalogFactory.java +++ b/core/trino-main/src/main/java/io/trino/connector/CatalogFactory.java @@ -18,12 +18,10 @@ import io.trino.spi.connector.Connector; import io.trino.spi.connector.ConnectorFactory; -import java.util.function.Function; - @ThreadSafe public interface CatalogFactory { - void addConnectorFactory(ConnectorFactory connectorFactory, Function duplicatePluginClassLoaderFactory); + void addConnectorFactory(ConnectorFactory connectorFactory); CatalogConnector createCatalog(CatalogProperties catalogProperties); diff --git a/core/trino-main/src/main/java/io/trino/connector/ConnectorContextInstance.java b/core/trino-main/src/main/java/io/trino/connector/ConnectorContextInstance.java index 677a4626ba95..e7da31cb64e9 100644 --- a/core/trino-main/src/main/java/io/trino/connector/ConnectorContextInstance.java +++ b/core/trino-main/src/main/java/io/trino/connector/ConnectorContextInstance.java @@ -24,10 +24,6 @@ import io.trino.spi.connector.MetadataProvider; import io.trino.spi.type.TypeManager; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.Supplier; - -import static com.google.common.base.Preconditions.checkState; import static java.util.Objects.requireNonNull; public class ConnectorContextInstance @@ -41,8 +37,6 @@ public class ConnectorContextInstance private final MetadataProvider metadataProvider; private final PageSorter pageSorter; private final PageIndexerFactory pageIndexerFactory; - private final Supplier duplicatePluginClassLoaderFactory; - private final AtomicBoolean pluginClassLoaderDuplicated = new AtomicBoolean(); private final CatalogHandle catalogHandle; public ConnectorContextInstance( @@ -54,8 +48,7 @@ public ConnectorContextInstance( TypeManager typeManager, MetadataProvider metadataProvider, PageSorter pageSorter, - PageIndexerFactory pageIndexerFactory, - Supplier duplicatePluginClassLoaderFactory) + PageIndexerFactory pageIndexerFactory) { this.openTelemetry = requireNonNull(openTelemetry, "openTelemetry is null"); this.tracer = requireNonNull(tracer, "tracer is null"); @@ -65,7 +58,6 @@ public ConnectorContextInstance( this.metadataProvider = requireNonNull(metadataProvider, "metadataProvider is null"); this.pageSorter = requireNonNull(pageSorter, "pageSorter is null"); this.pageIndexerFactory = requireNonNull(pageIndexerFactory, "pageIndexerFactory is null"); - this.duplicatePluginClassLoaderFactory = requireNonNull(duplicatePluginClassLoaderFactory, "duplicatePluginClassLoaderFactory is null"); this.catalogHandle = requireNonNull(catalogHandle, "catalogHandle is null"); } @@ -122,11 +114,4 @@ public PageIndexerFactory getPageIndexerFactory() { return pageIndexerFactory; } - - @Override - public ClassLoader duplicatePluginClassLoader() - { - checkState(!pluginClassLoaderDuplicated.getAndSet(true), "plugin class loader already duplicated"); - return duplicatePluginClassLoaderFactory.get(); - } } diff --git a/core/trino-main/src/main/java/io/trino/connector/ConnectorServices.java b/core/trino-main/src/main/java/io/trino/connector/ConnectorServices.java index fc1a4d5650ef..416e0bc0e049 100644 --- a/core/trino-main/src/main/java/io/trino/connector/ConnectorServices.java +++ b/core/trino-main/src/main/java/io/trino/connector/ConnectorServices.java @@ -72,7 +72,6 @@ public class ConnectorServices private final Tracer tracer; private final CatalogHandle catalogHandle; private final Connector connector; - private final Runnable afterShutdown; private final Set systemTables; private final CatalogProcedures procedures; private final CatalogTableProcedures tableProcedures; @@ -95,12 +94,11 @@ public class ConnectorServices private final AtomicBoolean shutdown = new AtomicBoolean(); - public ConnectorServices(Tracer tracer, CatalogHandle catalogHandle, Connector connector, Runnable afterShutdown) + public ConnectorServices(Tracer tracer, CatalogHandle catalogHandle, Connector connector) { this.tracer = requireNonNull(tracer, "tracer is null"); this.catalogHandle = requireNonNull(catalogHandle, "catalogHandle is null"); this.connector = requireNonNull(connector, "connector is null"); - this.afterShutdown = requireNonNull(afterShutdown, "afterShutdown is null"); Set systemTables = connector.getSystemTables(); requireNonNull(systemTables, format("Connector '%s' returned a null system tables set", catalogHandle)); @@ -345,9 +343,6 @@ public void shutdown() catch (Throwable t) { log.error(t, "Error shutting down catalog: %s", catalogHandle); } - finally { - afterShutdown.run(); - } } private static void validateTableFunction(ConnectorTableFunction tableFunction) diff --git a/core/trino-main/src/main/java/io/trino/connector/DefaultCatalogFactory.java b/core/trino-main/src/main/java/io/trino/connector/DefaultCatalogFactory.java index f544ef90e27a..121e8527830a 100644 --- a/core/trino-main/src/main/java/io/trino/connector/DefaultCatalogFactory.java +++ b/core/trino-main/src/main/java/io/trino/connector/DefaultCatalogFactory.java @@ -14,7 +14,6 @@ package io.trino.connector; import com.google.errorprone.annotations.ThreadSafe; -import com.google.errorprone.annotations.concurrent.GuardedBy; import com.google.inject.Inject; import io.airlift.node.NodeInfo; import io.opentelemetry.api.OpenTelemetry; @@ -25,11 +24,9 @@ import io.trino.connector.system.SystemConnector; import io.trino.connector.system.SystemTablesProvider; import io.trino.execution.scheduler.NodeSchedulerConfig; -import io.trino.metadata.HandleResolver; import io.trino.metadata.InternalNodeManager; import io.trino.metadata.Metadata; import io.trino.security.AccessControl; -import io.trino.server.PluginClassLoader; import io.trino.spi.PageIndexerFactory; import io.trino.spi.PageSorter; import io.trino.spi.VersionEmbedder; @@ -46,11 +43,8 @@ import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.function.Function; -import java.util.function.Supplier; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkState; import static io.trino.spi.connector.CatalogHandle.createInformationSchemaCatalogHandle; import static io.trino.spi.connector.CatalogHandle.createSystemTablesCatalogHandle; import static java.util.Objects.requireNonNull; @@ -61,7 +55,6 @@ public class DefaultCatalogFactory { private final Metadata metadata; private final AccessControl accessControl; - private final HandleResolver handleResolver; private final InternalNodeManager nodeManager; private final PageSorter pageSorter; @@ -75,13 +68,12 @@ public class DefaultCatalogFactory private final boolean schedulerIncludeCoordinator; private final int maxPrefetchedInformationSchemaPrefixes; - private final ConcurrentMap connectorFactories = new ConcurrentHashMap<>(); + private final ConcurrentMap connectorFactories = new ConcurrentHashMap<>(); @Inject public DefaultCatalogFactory( Metadata metadata, AccessControl accessControl, - HandleResolver handleResolver, InternalNodeManager nodeManager, PageSorter pageSorter, PageIndexerFactory pageIndexerFactory, @@ -95,7 +87,6 @@ public DefaultCatalogFactory( { this.metadata = requireNonNull(metadata, "metadata is null"); this.accessControl = requireNonNull(accessControl, "accessControl is null"); - this.handleResolver = requireNonNull(handleResolver, "handleResolver is null"); this.nodeManager = requireNonNull(nodeManager, "nodeManager is null"); this.pageSorter = requireNonNull(pageSorter, "pageSorter is null"); this.pageIndexerFactory = requireNonNull(pageIndexerFactory, "pageIndexerFactory is null"); @@ -109,11 +100,10 @@ public DefaultCatalogFactory( } @Override - public synchronized void addConnectorFactory(ConnectorFactory connectorFactory, Function duplicatePluginClassLoaderFactory) + public synchronized void addConnectorFactory(ConnectorFactory connectorFactory) { - InternalConnectorFactory existingConnectorFactory = connectorFactories.putIfAbsent( - new ConnectorName(connectorFactory.getName()), - new InternalConnectorFactory(connectorFactory, duplicatePluginClassLoaderFactory)); + ConnectorFactory existingConnectorFactory = connectorFactories.putIfAbsent( + new ConnectorName(connectorFactory.getName()), connectorFactory); checkArgument(existingConnectorFactory == null, "Connector '%s' is already registered", connectorFactory.getName()); } @@ -122,54 +112,43 @@ public CatalogConnector createCatalog(CatalogProperties catalogProperties) { requireNonNull(catalogProperties, "catalogProperties is null"); - InternalConnectorFactory factory = connectorFactories.get(catalogProperties.getConnectorName()); - checkArgument(factory != null, "No factory for connector '%s'. Available factories: %s", catalogProperties.getConnectorName(), connectorFactories.keySet()); + ConnectorFactory connectorFactory = connectorFactories.get(catalogProperties.getConnectorName()); + checkArgument(connectorFactory != null, "No factory for connector '%s'. Available factories: %s", catalogProperties.getConnectorName(), connectorFactories.keySet()); - CatalogClassLoaderSupplier duplicatePluginClassLoaderFactory = new CatalogClassLoaderSupplier( + Connector connector = createConnector( + catalogProperties.getCatalogHandle().getCatalogName(), catalogProperties.getCatalogHandle(), - factory.getDuplicatePluginClassLoaderFactory(), - handleResolver); - try { - Connector connector = createConnector( - catalogProperties.getCatalogHandle().getCatalogName(), - catalogProperties.getCatalogHandle(), - factory.getConnectorFactory(), - duplicatePluginClassLoaderFactory, - catalogProperties.getProperties()); - return createCatalog( - catalogProperties.getCatalogHandle(), - catalogProperties.getConnectorName(), - connector, - duplicatePluginClassLoaderFactory::destroy, - Optional.of(catalogProperties)); - } - catch (Throwable e) { - duplicatePluginClassLoaderFactory.destroy(); - throw e; - } + connectorFactory, + catalogProperties.getProperties()); + + return createCatalog( + catalogProperties.getCatalogHandle(), + catalogProperties.getConnectorName(), + connector, + Optional.of(catalogProperties)); } @Override public CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName connectorName, Connector connector) { - return createCatalog(catalogHandle, connectorName, connector, () -> {}, Optional.empty()); + return createCatalog(catalogHandle, connectorName, connector, Optional.empty()); } - private CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName connectorName, Connector connector, Runnable destroy, Optional catalogProperties) + private CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName connectorName, Connector connector, Optional catalogProperties) { Tracer tracer = createTracer(catalogHandle); - ConnectorServices catalogConnector = new ConnectorServices( - tracer, - catalogHandle, - connector, - destroy); + ConnectorServices catalogConnector = new ConnectorServices(tracer, catalogHandle, connector); ConnectorServices informationSchemaConnector = new ConnectorServices( tracer, createInformationSchemaCatalogHandle(catalogHandle), - new InformationSchemaConnector(catalogHandle.getCatalogName(), nodeManager, metadata, accessControl, maxPrefetchedInformationSchemaPrefixes), - () -> {}); + new InformationSchemaConnector( + catalogHandle.getCatalogName(), + nodeManager, + metadata, + accessControl, + maxPrefetchedInformationSchemaPrefixes)); SystemTablesProvider systemTablesProvider; if (nodeManager.getCurrentNode().isCoordinator()) { @@ -189,8 +168,7 @@ private CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorNam new SystemConnector( nodeManager, systemTablesProvider, - transactionId -> transactionManager.getConnectorTransaction(transactionId, catalogHandle)), - () -> {}); + transactionId -> transactionManager.getConnectorTransaction(transactionId, catalogHandle))); return new CatalogConnector( catalogHandle, @@ -205,7 +183,6 @@ private Connector createConnector( String catalogName, CatalogHandle catalogHandle, ConnectorFactory connectorFactory, - Supplier duplicatePluginClassLoaderFactory, Map properties) { ConnectorContext context = new ConnectorContextInstance( @@ -217,8 +194,7 @@ private Connector createConnector( typeManager, new InternalMetadataProvider(metadata, typeManager), pageSorter, - pageIndexerFactory, - duplicatePluginClassLoaderFactory); + pageIndexerFactory); try (ThreadContextClassLoader ignored = new ThreadContextClassLoader(connectorFactory.getClass().getClassLoader())) { return connectorFactory.create(catalogName, properties, context); @@ -229,87 +205,4 @@ private Tracer createTracer(CatalogHandle catalogHandle) { return openTelemetry.getTracer("trino.catalog." + catalogHandle.getCatalogName()); } - - private static class InternalConnectorFactory - { - private final ConnectorFactory connectorFactory; - private final Function duplicatePluginClassLoaderFactory; - - public InternalConnectorFactory(ConnectorFactory connectorFactory, Function duplicatePluginClassLoaderFactory) - { - this.connectorFactory = connectorFactory; - this.duplicatePluginClassLoaderFactory = duplicatePluginClassLoaderFactory; - } - - public ConnectorFactory getConnectorFactory() - { - return connectorFactory; - } - - public Function getDuplicatePluginClassLoaderFactory() - { - return duplicatePluginClassLoaderFactory; - } - - @Override - public String toString() - { - return connectorFactory.getName(); - } - } - - private static class CatalogClassLoaderSupplier - implements Supplier - { - private final CatalogHandle catalogHandle; - private final Function duplicatePluginClassLoaderFactory; - private final HandleResolver handleResolver; - - @GuardedBy("this") - private boolean destroyed; - - @GuardedBy("this") - private ClassLoader classLoader; - - public CatalogClassLoaderSupplier( - CatalogHandle catalogHandle, - Function duplicatePluginClassLoaderFactory, - HandleResolver handleResolver) - { - this.catalogHandle = requireNonNull(catalogHandle, "catalogHandle is null"); - this.duplicatePluginClassLoaderFactory = requireNonNull(duplicatePluginClassLoaderFactory, "duplicatePluginClassLoaderFactory is null"); - this.handleResolver = requireNonNull(handleResolver, "handleResolver is null"); - } - - @Override - public ClassLoader get() - { - ClassLoader classLoader = duplicatePluginClassLoaderFactory.apply(catalogHandle); - - synchronized (this) { - // we check this after class loader creation because it reduces the complexity of the synchronization, and this shouldn't happen - checkState(this.classLoader == null, "class loader is already a duplicated for catalog " + catalogHandle); - checkState(!destroyed, "catalog has been shutdown"); - this.classLoader = classLoader; - } - - if (classLoader instanceof PluginClassLoader) { - handleResolver.registerClassLoader((PluginClassLoader) classLoader); - } - return classLoader; - } - - public void destroy() - { - ClassLoader classLoader; - synchronized (this) { - checkState(!destroyed, "catalog has been shutdown"); - classLoader = this.classLoader; - destroyed = true; - } - if (classLoader instanceof PluginClassLoader) { - handleResolver.unregisterClassLoader((PluginClassLoader) classLoader); - } - } - } } diff --git a/core/trino-main/src/main/java/io/trino/connector/LazyCatalogFactory.java b/core/trino-main/src/main/java/io/trino/connector/LazyCatalogFactory.java index e3dadd0199a2..3d8d2964e181 100644 --- a/core/trino-main/src/main/java/io/trino/connector/LazyCatalogFactory.java +++ b/core/trino-main/src/main/java/io/trino/connector/LazyCatalogFactory.java @@ -18,7 +18,6 @@ import io.trino.spi.connector.ConnectorFactory; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Function; import static com.google.common.base.Preconditions.checkState; @@ -33,10 +32,9 @@ public void setCatalogFactory(CatalogFactory catalogFactory) } @Override - public void addConnectorFactory(ConnectorFactory connectorFactory, - Function duplicatePluginClassLoaderFactory) + public void addConnectorFactory(ConnectorFactory connectorFactory) { - getDelegate().addConnectorFactory(connectorFactory, duplicatePluginClassLoaderFactory); + getDelegate().addConnectorFactory(connectorFactory); } @Override diff --git a/core/trino-main/src/main/java/io/trino/metadata/HandleResolver.java b/core/trino-main/src/main/java/io/trino/metadata/HandleResolver.java index 67690b4b7cd3..c523183d1704 100644 --- a/core/trino-main/src/main/java/io/trino/metadata/HandleResolver.java +++ b/core/trino-main/src/main/java/io/trino/metadata/HandleResolver.java @@ -40,12 +40,6 @@ public void registerClassLoader(PluginClassLoader classLoader) checkState(existingClassLoader == null, "Class loader already registered: %s", classLoader.getId()); } - public void unregisterClassLoader(PluginClassLoader classLoader) - { - boolean result = classLoaders.remove(classLoader.getId(), classLoader); - checkState(result, "Class loader not registered: %s", classLoader.getId()); - } - public String getId(Object handle) { return classId(handle.getClass()); diff --git a/core/trino-main/src/main/java/io/trino/server/PluginClassLoader.java b/core/trino-main/src/main/java/io/trino/server/PluginClassLoader.java index 3311346378e3..370a0ae2e5f7 100644 --- a/core/trino-main/src/main/java/io/trino/server/PluginClassLoader.java +++ b/core/trino-main/src/main/java/io/trino/server/PluginClassLoader.java @@ -14,17 +14,14 @@ package io.trino.server; import com.google.common.collect.ImmutableList; -import io.trino.spi.connector.CatalogHandle; import java.io.IOException; import java.net.URL; import java.net.URLClassLoader; import java.util.Enumeration; import java.util.List; -import java.util.Optional; import static com.google.common.base.MoreObjects.toStringHelper; -import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; import static java.lang.System.identityHashCode; import static java.util.Objects.requireNonNull; @@ -32,9 +29,7 @@ public class PluginClassLoader extends URLClassLoader { - private final String id; private final String pluginName; - private final Optional catalogHandle; private final ClassLoader spiClassLoader; private final List spiPackages; private final List spiResources; @@ -46,7 +41,6 @@ public PluginClassLoader( List spiPackages) { this(pluginName, - Optional.empty(), urls, spiClassLoader, spiPackages, @@ -57,7 +51,6 @@ public PluginClassLoader( private PluginClassLoader( String pluginName, - Optional catalogHandle, List urls, ClassLoader spiClassLoader, Iterable spiPackages, @@ -66,34 +59,20 @@ private PluginClassLoader( // plugins should not have access to the system (application) class loader super(urls.toArray(new URL[0]), getPlatformClassLoader()); this.pluginName = requireNonNull(pluginName, "pluginName is null"); - this.catalogHandle = requireNonNull(catalogHandle, "catalogHandle is null"); this.spiClassLoader = requireNonNull(spiClassLoader, "spiClassLoader is null"); this.spiPackages = ImmutableList.copyOf(spiPackages); this.spiResources = ImmutableList.copyOf(spiResources); - this.id = pluginName + catalogHandle.map(name -> ":%s:%s".formatted(name.getCatalogName(), name.getVersion())).orElse(""); - } - - public PluginClassLoader duplicate(CatalogHandle catalogHandle) - { - checkState(this.catalogHandle.isEmpty(), "class loader is already a duplicate"); - return new PluginClassLoader( - pluginName, - Optional.of(requireNonNull(catalogHandle, "catalogHandle is null")), - ImmutableList.copyOf(getURLs()), - spiClassLoader, - spiPackages, - spiResources); } public PluginClassLoader withUrl(URL url) { List urls = ImmutableList.builder().add(getURLs()).add(url).build(); - return new PluginClassLoader(pluginName, catalogHandle, urls, spiClassLoader, spiPackages, spiResources); + return new PluginClassLoader(pluginName, urls, spiClassLoader, spiPackages, spiResources); } public String getId() { - return id; + return pluginName; } @Override @@ -102,7 +81,6 @@ public String toString() return toStringHelper(this) .omitNullValues() .add("plugin", pluginName) - .add("catalog", catalogHandle.orElse(null)) .add("identityHash", "@" + Integer.toHexString(identityHashCode(this))) .toString(); } diff --git a/core/trino-main/src/main/java/io/trino/server/PluginInstaller.java b/core/trino-main/src/main/java/io/trino/server/PluginInstaller.java index 452fa0f158ed..0cc6b2354110 100644 --- a/core/trino-main/src/main/java/io/trino/server/PluginInstaller.java +++ b/core/trino-main/src/main/java/io/trino/server/PluginInstaller.java @@ -14,13 +14,10 @@ package io.trino.server; import io.trino.spi.Plugin; -import io.trino.spi.connector.CatalogHandle; - -import java.util.function.Function; public interface PluginInstaller { void loadPlugins(); - void installPlugin(Plugin plugin, Function duplicatePluginClassLoaderFactory); + void installPlugin(Plugin plugin); } diff --git a/core/trino-main/src/main/java/io/trino/server/PluginManager.java b/core/trino-main/src/main/java/io/trino/server/PluginManager.java index c3d7cd866f80..4181aab50ea8 100644 --- a/core/trino-main/src/main/java/io/trino/server/PluginManager.java +++ b/core/trino-main/src/main/java/io/trino/server/PluginManager.java @@ -35,7 +35,6 @@ import io.trino.spi.Plugin; import io.trino.spi.block.BlockEncoding; import io.trino.spi.classloader.ThreadContextClassLoader; -import io.trino.spi.connector.CatalogHandle; import io.trino.spi.connector.ConnectorFactory; import io.trino.spi.eventlistener.EventListenerFactory; import io.trino.spi.exchange.ExchangeManagerFactory; @@ -55,7 +54,6 @@ import java.util.ServiceLoader; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.Function; import java.util.function.Supplier; import static com.google.common.base.Preconditions.checkState; @@ -168,18 +166,18 @@ private void loadPlugin(PluginClassLoader pluginClassLoader) for (Plugin plugin : plugins) { log.info("Installing %s", plugin.getClass().getName()); - installPlugin(plugin, pluginClassLoader::duplicate); + installPlugin(plugin); } } @Override - public void installPlugin(Plugin plugin, Function duplicatePluginClassLoaderFactory) + public void installPlugin(Plugin plugin) { - installPluginInternal(plugin, duplicatePluginClassLoaderFactory); + installPluginInternal(plugin); typeRegistry.verifyTypes(); } - private void installPluginInternal(Plugin plugin, Function duplicatePluginClassLoaderFactory) + private void installPluginInternal(Plugin plugin) { for (BlockEncoding blockEncoding : plugin.getBlockEncodings()) { log.info("Registering block encoding %s", blockEncoding.getName()); @@ -198,7 +196,7 @@ private void installPluginInternal(Plugin plugin, Function> functions = plugin.getFunctions(); diff --git a/core/trino-main/src/main/java/io/trino/server/testing/TestingTrinoServer.java b/core/trino-main/src/main/java/io/trino/server/testing/TestingTrinoServer.java index b2da13f8aa15..672b7497b352 100644 --- a/core/trino-main/src/main/java/io/trino/server/testing/TestingTrinoServer.java +++ b/core/trino-main/src/main/java/io/trino/server/testing/TestingTrinoServer.java @@ -428,7 +428,7 @@ public void close() public void installPlugin(Plugin plugin) { - pluginInstaller.installPlugin(plugin, ignored -> plugin.getClass().getClassLoader()); + pluginInstaller.installPlugin(plugin); } public DispatchManager getDispatchManager() diff --git a/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java b/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java index b7cdd40c0839..cf607bef7119 100644 --- a/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java +++ b/core/trino-main/src/main/java/io/trino/testing/LocalQueryRunner.java @@ -394,13 +394,10 @@ private LocalQueryRunner( this.accessControl = new TestingAccessControlManager(transactionManager, eventListenerManager); accessControl.loadSystemAccessControl(AllowAllSystemAccessControl.NAME, ImmutableMap.of()); - HandleResolver handleResolver = new HandleResolver(); - NodeInfo nodeInfo = new NodeInfo("test"); catalogFactory.setCatalogFactory(new DefaultCatalogFactory( metadata, accessControl, - handleResolver, nodeManager, pageSorter, pageIndexerFactory, @@ -490,7 +487,7 @@ private LocalQueryRunner( new SessionPropertyDefaults(nodeInfo, accessControl), typeRegistry, blockEncodingManager, - handleResolver, + new HandleResolver(), exchangeManagerRegistry); catalogManager.registerGlobalSystemConnector(globalSystemConnector); @@ -757,19 +754,19 @@ public ExpressionCompiler getExpressionCompiler() public void createCatalog(String catalogName, ConnectorFactory connectorFactory, Map properties) { - catalogFactory.addConnectorFactory(connectorFactory, ignored -> connectorFactory.getClass().getClassLoader()); + catalogFactory.addConnectorFactory(connectorFactory); catalogManager.createCatalog(catalogName, new ConnectorName(connectorFactory.getName()), properties, false); } public void registerCatalogFactory(ConnectorFactory connectorFactory) { - catalogFactory.addConnectorFactory(connectorFactory, ignored -> connectorFactory.getClass().getClassLoader()); + catalogFactory.addConnectorFactory(connectorFactory); } @Override public void installPlugin(Plugin plugin) { - pluginManager.installPlugin(plugin, ignored -> plugin.getClass().getClassLoader()); + pluginManager.installPlugin(plugin); } @Override diff --git a/core/trino-main/src/main/java/io/trino/testing/TestingConnectorContext.java b/core/trino-main/src/main/java/io/trino/testing/TestingConnectorContext.java index 0d096a44235b..d7ff18054488 100644 --- a/core/trino-main/src/main/java/io/trino/testing/TestingConnectorContext.java +++ b/core/trino-main/src/main/java/io/trino/testing/TestingConnectorContext.java @@ -104,10 +104,4 @@ public PageIndexerFactory getPageIndexerFactory() { return pageIndexerFactory; } - - @Override - public ClassLoader duplicatePluginClassLoader() - { - return getClass().getClassLoader(); - } } diff --git a/core/trino-main/src/test/java/io/trino/execution/TestSqlTaskManagerRaceWithCatalogPrune.java b/core/trino-main/src/test/java/io/trino/execution/TestSqlTaskManagerRaceWithCatalogPrune.java index 815c8dd85a8d..7fe6e9e4b997 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestSqlTaskManagerRaceWithCatalogPrune.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestSqlTaskManagerRaceWithCatalogPrune.java @@ -71,7 +71,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.DoubleSupplier; -import java.util.function.Function; import java.util.function.Predicate; import static io.airlift.tracing.Tracing.noopTracer; @@ -112,7 +111,7 @@ public ConnectorServices getConnectorServices(CatalogHandle catalogHandle) private static final CatalogFactory MOCK_CATALOG_FACTORY = new CatalogFactory() { @Override - public void addConnectorFactory(ConnectorFactory connectorFactory, Function duplicatePluginClassLoaderFactory) {} + public void addConnectorFactory(ConnectorFactory connectorFactory) {} @Override public CatalogConnector createCatalog(CatalogProperties catalogProperties) @@ -121,8 +120,7 @@ public CatalogConnector createCatalog(CatalogProperties catalogProperties) ConnectorServices noOpConnectorService = new ConnectorServices( Tracing.noopTracer(), catalogProperties.getCatalogHandle(), - connector, - () -> {}); + connector); return new CatalogConnector( catalogProperties.getCatalogHandle(), new ConnectorName("mock"), diff --git a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorContext.java b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorContext.java index db8c9d653b98..c009e49d7794 100644 --- a/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorContext.java +++ b/core/trino-spi/src/main/java/io/trino/spi/connector/ConnectorContext.java @@ -78,10 +78,4 @@ default PageIndexerFactory getPageIndexerFactory() { throw new UnsupportedOperationException(); } - - @Deprecated(forRemoval = true) - default ClassLoader duplicatePluginClassLoader() - { - throw new UnsupportedOperationException(); - } }