From 73dc71e91e630c9d13931199c6e7038d80731679 Mon Sep 17 00:00:00 2001 From: Dain Sundstrom Date: Fri, 14 Apr 2023 18:13:41 -0700 Subject: [PATCH 1/2] Destroy duplicate class loaders if catalog creation fails --- .../connector/DefaultCatalogFactory.java | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) 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 19d958f5a650..b127c7efb204 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 @@ -121,18 +121,24 @@ public CatalogConnector createCatalog(CatalogProperties catalogProperties) catalogProperties.getCatalogHandle(), factory.getDuplicatePluginClassLoaderFactory(), handleResolver); - 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)); + 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; + } } @Override From 7263c66a47af3ca65a771e50d85230ff56807e94 Mon Sep 17 00:00:00 2001 From: Dain Sundstrom Date: Fri, 14 Apr 2023 18:17:45 -0700 Subject: [PATCH 2/2] Add catalog version to plugin class loader id --- .../src/main/java/io/trino/server/PluginClassLoader.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 1cc512978477..3311346378e3 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 @@ -32,6 +32,7 @@ public class PluginClassLoader extends URLClassLoader { + private final String id; private final String pluginName; private final Optional catalogHandle; private final ClassLoader spiClassLoader; @@ -69,6 +70,7 @@ private PluginClassLoader( 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) @@ -91,7 +93,7 @@ public PluginClassLoader withUrl(URL url) public String getId() { - return pluginName + catalogHandle.map(name -> ":" + name).orElse(""); + return id; } @Override