github-actions[bot] commented on code in PR #66708: URL: https://github.com/apache/doris/pull/66708#discussion_r3772111802
########## fe/fe-core/src/main/java/org/apache/doris/common/plugin/CloudPluginDownloader.java: ########## @@ -0,0 +1,152 @@ +// 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.doris.common.plugin; + +import org.apache.doris.cloud.proto.Cloud; +import org.apache.doris.cloud.rpc.MetaServiceProxy; +import org.apache.doris.cloud.storage.ObjectInfo; +import org.apache.doris.cloud.storage.ObjectInfoAdapter; +import org.apache.doris.datasource.storage.StorageAdapter; +import org.apache.doris.filesystem.FileSystem; +import org.apache.doris.filesystem.Location; +import org.apache.doris.fs.FileSystemFactory; +import org.apache.doris.service.FrontendOptions; + +import com.google.common.base.Strings; + +import java.io.File; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; + +/** + * Simple cloud plugin downloader for UDF and JDBC drivers. + */ +public class CloudPluginDownloader { + + public enum PluginType { + JDBC_DRIVERS, + JAVA_UDF, + CONNECTORS, // Reserved, not supported yet + HADOOP_CONF // Reserved, not supported yet + } + + /** + * Download plugin from cloud storage to local path + */ + public static synchronized String downloadFromCloud(PluginType type, String name, String localPath) { + validateInput(type, name); + try { + Cloud.ObjectStoreInfoPB objInfo = getCloudStorageInfo(); + String remotePath = buildS3Path(objInfo, type, name); + return doDownload(objInfo, remotePath, localPath); + } catch (Exception e) { + throw new RuntimeException("Failed to download plugin: " + e.getMessage(), e); + } + } + + /** + * Validate input parameters + */ + static void validateInput(PluginType type, String name) { + if (Strings.isNullOrEmpty(name)) { + throw new IllegalArgumentException("Plugin name cannot be empty"); + } + + if (type != PluginType.JDBC_DRIVERS && type != PluginType.JAVA_UDF) { + throw new UnsupportedOperationException("Plugin type " + type + " is not supported yet"); + } + } + + /** + * Get cloud storage info from MetaService + * Package-private for testing + */ + static Cloud.ObjectStoreInfoPB getCloudStorageInfo() throws Exception { + Cloud.GetObjStoreInfoResponse response = MetaServiceProxy.getInstance() + .getObjStoreInfo(Cloud.GetObjStoreInfoRequest.newBuilder() + .setRequestIp(FrontendOptions.getLocalHostAddressCached()) + .build()); + + if (response.getStatus().getCode() != Cloud.MetaServiceCode.OK) { + throw new RuntimeException("Failed to get storage info: " + response.getStatus().getMsg()); + } + + if (response.getObjInfoList().isEmpty()) { + throw new RuntimeException("Only SaaS cloud storage is supported currently"); + } + + return response.getObjInfo(0); + } + + /** + * Build complete S3 path from objInfo + * Package-private for testing + */ + static String buildS3Path(Cloud.ObjectStoreInfoPB objInfo, PluginType type, String name) { + String bucket = objInfo.getBucket(); + String prefix = objInfo.hasPrefix() ? objInfo.getPrefix() : ""; + String relativePath = String.format("plugins/%s/%s", type.name().toLowerCase(), name); Review Comment: Use a locale-independent artifact directory here. With a Turkish default locale, `JDBC_DRIVERS.toLowerCase()` becomes `jdbc_drıvers` (dotless i), while BE always requests the fixed ASCII `plugins/jdbc_drivers/...`; FE then either gets a 404 or checksums different bytes from BE. Prefer an explicit per-enum directory mapping matching the BE switch, or at least `Locale.ROOT`. ########## fe/fe-core/src/main/java/org/apache/doris/common/plugin/CloudPluginDownloader.java: ########## @@ -0,0 +1,152 @@ +// 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.doris.common.plugin; + +import org.apache.doris.cloud.proto.Cloud; +import org.apache.doris.cloud.rpc.MetaServiceProxy; +import org.apache.doris.cloud.storage.ObjectInfo; +import org.apache.doris.cloud.storage.ObjectInfoAdapter; +import org.apache.doris.datasource.storage.StorageAdapter; +import org.apache.doris.filesystem.FileSystem; +import org.apache.doris.filesystem.Location; +import org.apache.doris.fs.FileSystemFactory; +import org.apache.doris.service.FrontendOptions; + +import com.google.common.base.Strings; + +import java.io.File; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; + +/** + * Simple cloud plugin downloader for UDF and JDBC drivers. + */ +public class CloudPluginDownloader { + + public enum PluginType { + JDBC_DRIVERS, + JAVA_UDF, + CONNECTORS, // Reserved, not supported yet + HADOOP_CONF // Reserved, not supported yet + } + + /** + * Download plugin from cloud storage to local path + */ + public static synchronized String downloadFromCloud(PluginType type, String name, String localPath) { Review Comment: This class-wide monitor is held through the MetaService RPC, filesystem/client construction, and the complete remote copy. A slow UDF download therefore blocks CREATE CATALOG (and every other unrelated plugin name) on this FE for the full transfer time. Coordinate per normalized target and combine that with atomic publication; independent network I/O should not sit under a global monitor. ########## fe/fe-core/src/main/java/org/apache/doris/common/plugin/CloudPluginDownloader.java: ########## @@ -0,0 +1,152 @@ +// 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.doris.common.plugin; + +import org.apache.doris.cloud.proto.Cloud; +import org.apache.doris.cloud.rpc.MetaServiceProxy; +import org.apache.doris.cloud.storage.ObjectInfo; +import org.apache.doris.cloud.storage.ObjectInfoAdapter; +import org.apache.doris.datasource.storage.StorageAdapter; +import org.apache.doris.filesystem.FileSystem; +import org.apache.doris.filesystem.Location; +import org.apache.doris.fs.FileSystemFactory; +import org.apache.doris.service.FrontendOptions; + +import com.google.common.base.Strings; + +import java.io.File; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; + +/** + * Simple cloud plugin downloader for UDF and JDBC drivers. + */ +public class CloudPluginDownloader { + + public enum PluginType { + JDBC_DRIVERS, + JAVA_UDF, + CONNECTORS, // Reserved, not supported yet + HADOOP_CONF // Reserved, not supported yet + } + + /** + * Download plugin from cloud storage to local path + */ + public static synchronized String downloadFromCloud(PluginType type, String name, String localPath) { + validateInput(type, name); + try { + Cloud.ObjectStoreInfoPB objInfo = getCloudStorageInfo(); + String remotePath = buildS3Path(objInfo, type, name); + return doDownload(objInfo, remotePath, localPath); + } catch (Exception e) { + throw new RuntimeException("Failed to download plugin: " + e.getMessage(), e); + } + } + + /** + * Validate input parameters + */ + static void validateInput(PluginType type, String name) { + if (Strings.isNullOrEmpty(name)) { + throw new IllegalArgumentException("Plugin name cannot be empty"); + } + + if (type != PluginType.JDBC_DRIVERS && type != PluginType.JAVA_UDF) { + throw new UnsupportedOperationException("Plugin type " + type + " is not supported yet"); + } + } + + /** + * Get cloud storage info from MetaService + * Package-private for testing + */ + static Cloud.ObjectStoreInfoPB getCloudStorageInfo() throws Exception { + Cloud.GetObjStoreInfoResponse response = MetaServiceProxy.getInstance() + .getObjStoreInfo(Cloud.GetObjStoreInfoRequest.newBuilder() + .setRequestIp(FrontendOptions.getLocalHostAddressCached()) + .build()); + + if (response.getStatus().getCode() != Cloud.MetaServiceCode.OK) { + throw new RuntimeException("Failed to get storage info: " + response.getStatus().getMsg()); + } + + if (response.getObjInfoList().isEmpty()) { + throw new RuntimeException("Only SaaS cloud storage is supported currently"); + } + + return response.getObjInfo(0); + } + + /** + * Build complete S3 path from objInfo + * Package-private for testing + */ + static String buildS3Path(Cloud.ObjectStoreInfoPB objInfo, PluginType type, String name) { + String bucket = objInfo.getBucket(); + String prefix = objInfo.hasPrefix() ? objInfo.getPrefix() : ""; + String relativePath = String.format("plugins/%s/%s", type.name().toLowerCase(), name); + + String fullPath; + if (Strings.isNullOrEmpty(prefix)) { Review Comment: Define one portable artifact-name/key contract before building this raw URI. Azure's parser strips `?`/`#` and percent-decodes the key, while BE passes `plugins/java_udf/<name>` through as a raw relative path. An accepted name such as `udf#v2.jar` therefore makes FE request `udf` and BE request the literal suffixed key. Enforce the same grammar/encoding on both sides and add Azure parity cases for reserved characters. ########## fe/fe-core/src/test/java/org/apache/doris/common/plugin/CloudPluginDownloaderTest.java: ########## @@ -0,0 +1,173 @@ +// 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.doris.common.plugin; + +import org.apache.doris.cloud.proto.Cloud; +import org.apache.doris.cloud.rpc.MetaServiceProxy; +import org.apache.doris.common.plugin.CloudPluginDownloader.PluginType; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; +import org.mockito.Mockito; + +import java.util.Collections; + +/** + * Unit tests for CloudPluginDownloader using package-private methods for direct white-box testing. + */ +public class CloudPluginDownloaderTest { + + private Cloud.GetObjStoreInfoResponse mockResponse; + private Cloud.ObjectStoreInfoPB mockObjInfo; + private MetaServiceProxy mockMetaServiceProxy; + + @BeforeEach + void setUp() { + mockResponse = Mockito.mock(Cloud.GetObjStoreInfoResponse.class); + mockObjInfo = Mockito.mock(Cloud.ObjectStoreInfoPB.class); + mockMetaServiceProxy = Mockito.mock(MetaServiceProxy.class); + } + + // ============== validateInput Tests ============== + + @Test + void testValidateInput() { + // Positive cases + Assertions.assertDoesNotThrow(() -> { + CloudPluginDownloader.validateInput(PluginType.JDBC_DRIVERS, "mysql.jar"); + CloudPluginDownloader.validateInput(PluginType.JAVA_UDF, "my_udf.jar"); + }); + + // Empty/null name + IllegalArgumentException ex1 = Assertions.assertThrows(IllegalArgumentException.class, + () -> CloudPluginDownloader.validateInput(PluginType.JDBC_DRIVERS, "")); + Assertions.assertEquals("Plugin name cannot be empty", ex1.getMessage()); + + IllegalArgumentException ex2 = Assertions.assertThrows(IllegalArgumentException.class, + () -> CloudPluginDownloader.validateInput(PluginType.JDBC_DRIVERS, null)); + Assertions.assertEquals("Plugin name cannot be empty", ex2.getMessage()); + + // Unsupported types + UnsupportedOperationException ex3 = Assertions.assertThrows(UnsupportedOperationException.class, + () -> CloudPluginDownloader.validateInput(PluginType.CONNECTORS, "test.jar")); + Assertions.assertTrue(ex3.getMessage().contains("is not supported yet")); + } + + // ============== getCloudStorageInfo Tests ============== + + @Test + void testGetCloudStorageInfo() throws Exception { + try (MockedStatic<MetaServiceProxy> mockedStatic = Mockito.mockStatic(MetaServiceProxy.class)) { + mockedStatic.when(MetaServiceProxy::getInstance).thenReturn(mockMetaServiceProxy); + + // Success case + Cloud.MetaServiceResponseStatus okStatus = Cloud.MetaServiceResponseStatus.newBuilder() + .setCode(Cloud.MetaServiceCode.OK).build(); + Mockito.when(mockResponse.getStatus()).thenReturn(okStatus); + Mockito.when(mockResponse.getObjInfoList()).thenReturn(Collections.singletonList(mockObjInfo)); + Mockito.when(mockResponse.getObjInfo(0)).thenReturn(mockObjInfo); + Mockito.when(mockMetaServiceProxy.getObjStoreInfo(Mockito.any())).thenReturn(mockResponse); + + Cloud.ObjectStoreInfoPB result = CloudPluginDownloader.getCloudStorageInfo(); + Assertions.assertEquals(mockObjInfo, result); + + // Error response + Cloud.MetaServiceResponseStatus failedStatus = Cloud.MetaServiceResponseStatus.newBuilder() + .setCode(Cloud.MetaServiceCode.INVALID_ARGUMENT).setMsg("Test error").build(); + Mockito.when(mockResponse.getStatus()).thenReturn(failedStatus); + + RuntimeException ex1 = Assertions.assertThrows(RuntimeException.class, + CloudPluginDownloader::getCloudStorageInfo); + Assertions.assertTrue(ex1.getMessage().contains("Failed to get storage info")); + + // Empty storage list + Mockito.when(mockResponse.getStatus()).thenReturn(okStatus); + Mockito.when(mockResponse.getObjInfoList()).thenReturn(Collections.emptyList()); + + RuntimeException ex2 = Assertions.assertThrows(RuntimeException.class, + CloudPluginDownloader::getCloudStorageInfo); + Assertions.assertTrue(ex2.getMessage().contains("Only SaaS cloud storage is supported")); + } + } + + // ============== buildS3Path Tests ============== + + @Test + void testBuildS3Path() { + Mockito.when(mockObjInfo.getBucket()).thenReturn("test-bucket"); + + // With prefix + Mockito.when(mockObjInfo.hasPrefix()).thenReturn(true); + Mockito.when(mockObjInfo.getPrefix()).thenReturn("test-prefix"); + Assertions.assertEquals("s3://test-bucket/test-prefix/plugins/jdbc_drivers/mysql.jar", + CloudPluginDownloader.buildS3Path(mockObjInfo, PluginType.JDBC_DRIVERS, "mysql.jar")); + + // Without prefix + Mockito.when(mockObjInfo.hasPrefix()).thenReturn(false); + Assertions.assertEquals("s3://test-bucket/plugins/java_udf/my_udf.jar", + CloudPluginDownloader.buildS3Path(mockObjInfo, PluginType.JAVA_UDF, "my_udf.jar")); + + // All plugin types + Assertions.assertEquals("s3://test-bucket/plugins/connectors/test.jar", + CloudPluginDownloader.buildS3Path(mockObjInfo, PluginType.CONNECTORS, "test.jar")); + Assertions.assertEquals("s3://test-bucket/plugins/hadoop_conf/test.xml", + CloudPluginDownloader.buildS3Path(mockObjInfo, PluginType.HADOOP_CONF, "test.xml")); + } + + // ============== Integration Test ============== + + @Test + void testDownloadFromCloudIntegration() { + // Basic integration test - should fail early due to validation + IllegalArgumentException ex = Assertions.assertThrows(IllegalArgumentException.class, + () -> CloudPluginDownloader.downloadFromCloud(PluginType.JDBC_DRIVERS, "", "/tmp/test.jar")); + Assertions.assertEquals("Plugin name cannot be empty", ex.getMessage()); + + // Should fail at MetaService level (no real cloud environment) + RuntimeException ex2 = Assertions.assertThrows(RuntimeException.class, + () -> CloudPluginDownloader.downloadFromCloud(PluginType.JDBC_DRIVERS, "mysql.jar", "/tmp/test.jar")); Review Comment: This test does not exercise the integration path: the first call stops at validation and the second contacts the process-global MetaService only to expect any failure. It is therefore environment-dependent and leaves `doDownload` (successful bytes, stream/filesystem closure, and mid-copy cleanup) untested. Add an injectable/fake filesystem seam and cover a successful copy plus injected stream failure/concurrency without external state. ########## fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java: ########## @@ -447,6 +449,18 @@ private static String checkAndReturnDefaultDriverUrl(String driverUrl) { } else if (oldTargetFile.exists()) { // File exists in old default directory return "file://" + oldTargetPath; + } else if (Config.isCloudMode()) { + // Cloud mode: download from cloud to default directory + try { + String downloadedPath = CloudPluginDownloader.downloadFromCloud( Review Comment: Make this materialization available on lazy/replay initialization, not only during leader-side CREATE validation. The catalog persists the bare `driver_url` plus checksum, replay skips `checkWhenCreating`, and `JdbcDorisConnector.createClient()` later uses a separate local-only resolver. A cache-empty follower promoted after replay therefore tries a nonexistent `file://.../jdbc_drivers/<name>` and cannot initialize the catalog even though the cloud object exists. Add a create-on-FE1, replay/promote-FE2 test. ########## fe/fe-core/src/main/java/org/apache/doris/common/plugin/CloudPluginDownloader.java: ########## @@ -0,0 +1,152 @@ +// 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.doris.common.plugin; + +import org.apache.doris.cloud.proto.Cloud; +import org.apache.doris.cloud.rpc.MetaServiceProxy; +import org.apache.doris.cloud.storage.ObjectInfo; +import org.apache.doris.cloud.storage.ObjectInfoAdapter; +import org.apache.doris.datasource.storage.StorageAdapter; +import org.apache.doris.filesystem.FileSystem; +import org.apache.doris.filesystem.Location; +import org.apache.doris.fs.FileSystemFactory; +import org.apache.doris.service.FrontendOptions; + +import com.google.common.base.Strings; + +import java.io.File; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; + +/** + * Simple cloud plugin downloader for UDF and JDBC drivers. + */ +public class CloudPluginDownloader { + + public enum PluginType { + JDBC_DRIVERS, + JAVA_UDF, + CONNECTORS, // Reserved, not supported yet + HADOOP_CONF // Reserved, not supported yet + } + + /** + * Download plugin from cloud storage to local path + */ + public static synchronized String downloadFromCloud(PluginType type, String name, String localPath) { + validateInput(type, name); + try { + Cloud.ObjectStoreInfoPB objInfo = getCloudStorageInfo(); + String remotePath = buildS3Path(objInfo, type, name); + return doDownload(objInfo, remotePath, localPath); + } catch (Exception e) { + throw new RuntimeException("Failed to download plugin: " + e.getMessage(), e); + } + } + + /** + * Validate input parameters + */ + static void validateInput(PluginType type, String name) { + if (Strings.isNullOrEmpty(name)) { + throw new IllegalArgumentException("Plugin name cannot be empty"); + } + + if (type != PluginType.JDBC_DRIVERS && type != PluginType.JAVA_UDF) { + throw new UnsupportedOperationException("Plugin type " + type + " is not supported yet"); + } + } + + /** + * Get cloud storage info from MetaService + * Package-private for testing + */ + static Cloud.ObjectStoreInfoPB getCloudStorageInfo() throws Exception { + Cloud.GetObjStoreInfoResponse response = MetaServiceProxy.getInstance() + .getObjStoreInfo(Cloud.GetObjStoreInfoRequest.newBuilder() + .setRequestIp(FrontendOptions.getLocalHostAddressCached()) + .build()); + + if (response.getStatus().getCode() != Cloud.MetaServiceCode.OK) { + throw new RuntimeException("Failed to get storage info: " + response.getStatus().getMsg()); + } + + if (response.getObjInfoList().isEmpty()) { + throw new RuntimeException("Only SaaS cloud storage is supported currently"); + } + + return response.getObjInfo(0); Review Comment: Select the same active legacy object-store entry as the rest of the cloud stack. `obj_info` is append-ordered history: MetaService returns it unchanged, current stage code treats the last element as latest, and BE publishes `vault_infos.back()` as `latest_fs`. On `[old, replacement]`, this FE reads entry 0 while BE downloads from the replacement, causing CREATE to 404 or persist an MD5 for different bytes. Use the last entry and add a multi-entry parity test. ########## fe/fe-core/src/main/java/org/apache/doris/common/plugin/CloudPluginDownloader.java: ########## @@ -0,0 +1,152 @@ +// 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.doris.common.plugin; + +import org.apache.doris.cloud.proto.Cloud; +import org.apache.doris.cloud.rpc.MetaServiceProxy; +import org.apache.doris.cloud.storage.ObjectInfo; +import org.apache.doris.cloud.storage.ObjectInfoAdapter; +import org.apache.doris.datasource.storage.StorageAdapter; +import org.apache.doris.filesystem.FileSystem; +import org.apache.doris.filesystem.Location; +import org.apache.doris.fs.FileSystemFactory; +import org.apache.doris.service.FrontendOptions; + +import com.google.common.base.Strings; + +import java.io.File; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; + +/** + * Simple cloud plugin downloader for UDF and JDBC drivers. + */ +public class CloudPluginDownloader { + + public enum PluginType { + JDBC_DRIVERS, + JAVA_UDF, + CONNECTORS, // Reserved, not supported yet + HADOOP_CONF // Reserved, not supported yet + } + + /** + * Download plugin from cloud storage to local path + */ + public static synchronized String downloadFromCloud(PluginType type, String name, String localPath) { + validateInput(type, name); + try { + Cloud.ObjectStoreInfoPB objInfo = getCloudStorageInfo(); + String remotePath = buildS3Path(objInfo, type, name); + return doDownload(objInfo, remotePath, localPath); + } catch (Exception e) { + throw new RuntimeException("Failed to download plugin: " + e.getMessage(), e); + } + } + + /** + * Validate input parameters + */ + static void validateInput(PluginType type, String name) { + if (Strings.isNullOrEmpty(name)) { + throw new IllegalArgumentException("Plugin name cannot be empty"); + } + + if (type != PluginType.JDBC_DRIVERS && type != PluginType.JAVA_UDF) { + throw new UnsupportedOperationException("Plugin type " + type + " is not supported yet"); + } + } + + /** + * Get cloud storage info from MetaService + * Package-private for testing + */ + static Cloud.ObjectStoreInfoPB getCloudStorageInfo() throws Exception { + Cloud.GetObjStoreInfoResponse response = MetaServiceProxy.getInstance() + .getObjStoreInfo(Cloud.GetObjStoreInfoRequest.newBuilder() + .setRequestIp(FrontendOptions.getLocalHostAddressCached()) + .build()); + + if (response.getStatus().getCode() != Cloud.MetaServiceCode.OK) { + throw new RuntimeException("Failed to get storage info: " + response.getStatus().getMsg()); + } + + if (response.getObjInfoList().isEmpty()) { + throw new RuntimeException("Only SaaS cloud storage is supported currently"); + } + + return response.getObjInfo(0); + } + + /** + * Build complete S3 path from objInfo + * Package-private for testing + */ + static String buildS3Path(Cloud.ObjectStoreInfoPB objInfo, PluginType type, String name) { + String bucket = objInfo.getBucket(); + String prefix = objInfo.hasPrefix() ? objInfo.getPrefix() : ""; + String relativePath = String.format("plugins/%s/%s", type.name().toLowerCase(), name); + + String fullPath; + if (Strings.isNullOrEmpty(prefix)) { + fullPath = bucket + "/" + relativePath; + } else { + fullPath = bucket + "/" + prefix + "/" + relativePath; + } + + return "s3://" + fullPath; + } + + /** + * Execute download using SPI FileSystem + */ + private static String doDownload(Cloud.ObjectStoreInfoPB objInfo, String remotePath, String localPath) + throws Exception { + // Create parent directory + Path parentDir = Paths.get(localPath).getParent(); + if (parentDir != null && !Files.exists(parentDir)) { + Files.createDirectories(parentDir); + } + + // Delete existing file if present + File localFile = new File(localPath); + if (localFile.exists() && !localFile.delete()) { + throw new RuntimeException("Failed to delete existing file: " + localPath); + } + + // Bind the provider reported by MetaService explicitly. Raw property auto-detection is + // order-dependent when more than one storage provider recognizes the same map. + StorageAdapter storageAdapter = createStorageAdapter(objInfo); + try (FileSystem fileSystem = FileSystemFactory.getFileSystem(storageAdapter); + InputStream in = fileSystem.newInputFile(Location.of(remotePath)).newStream()) { + Files.copy(in, localFile.toPath(), StandardCopyOption.REPLACE_EXISTING); + } + return localPath; + } + + /** + * Bind cloud object-store information to the provider selected by MetaService. + * Package-private for testing. + */ + static StorageAdapter createStorageAdapter(Cloud.ObjectStoreInfoPB objInfo) { + return ObjectInfoAdapter.toStorageAdapter(new ObjectInfo(objInfo)); Review Comment: Preserve all access-affecting fields from `ObjectStoreInfoPB` when building this adapter. The protobuf carries `use_path_style` and `cred_provider_type`, and BE consumes both, but `new ObjectInfo(objInfo)` drops them so the FE SPI silently falls back to virtual-host addressing and the DEFAULT credential chain. A valid path-style endpoint (or an explicit ENV/CONTAINER/WEB_IDENTITY source for assume-role) can work on BE but fail here. Extend the adapter test to assert these typed values too. ########## fe/fe-core/src/main/java/org/apache/doris/common/plugin/CloudPluginDownloader.java: ########## @@ -0,0 +1,152 @@ +// 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.doris.common.plugin; + +import org.apache.doris.cloud.proto.Cloud; +import org.apache.doris.cloud.rpc.MetaServiceProxy; +import org.apache.doris.cloud.storage.ObjectInfo; +import org.apache.doris.cloud.storage.ObjectInfoAdapter; +import org.apache.doris.datasource.storage.StorageAdapter; +import org.apache.doris.filesystem.FileSystem; +import org.apache.doris.filesystem.Location; +import org.apache.doris.fs.FileSystemFactory; +import org.apache.doris.service.FrontendOptions; + +import com.google.common.base.Strings; + +import java.io.File; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; + +/** + * Simple cloud plugin downloader for UDF and JDBC drivers. + */ +public class CloudPluginDownloader { + + public enum PluginType { + JDBC_DRIVERS, + JAVA_UDF, + CONNECTORS, // Reserved, not supported yet + HADOOP_CONF // Reserved, not supported yet + } + + /** + * Download plugin from cloud storage to local path + */ + public static synchronized String downloadFromCloud(PluginType type, String name, String localPath) { + validateInput(type, name); + try { + Cloud.ObjectStoreInfoPB objInfo = getCloudStorageInfo(); + String remotePath = buildS3Path(objInfo, type, name); + return doDownload(objInfo, remotePath, localPath); + } catch (Exception e) { + throw new RuntimeException("Failed to download plugin: " + e.getMessage(), e); + } + } + + /** + * Validate input parameters + */ + static void validateInput(PluginType type, String name) { Review Comment: Reject paths that escape the plugin directory before constructing or mutating the local target. A scheme-less UDF FILE such as `../jdbc_drivers/x.jar` reaches this method, becomes `<DORIS_HOME>/plugins/java_udf/../jdbc_drivers/x.jar`, and `doDownload` deletes that existing file before it even opens the remote object. Normalize the requested relative name against the per-type base and reject absolute/out-of-base results; apply the same contract on BE while preserving legitimate nested paths. ########## fe/fe-core/src/main/java/org/apache/doris/common/plugin/CloudPluginDownloader.java: ########## @@ -0,0 +1,152 @@ +// 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.doris.common.plugin; + +import org.apache.doris.cloud.proto.Cloud; +import org.apache.doris.cloud.rpc.MetaServiceProxy; +import org.apache.doris.cloud.storage.ObjectInfo; +import org.apache.doris.cloud.storage.ObjectInfoAdapter; +import org.apache.doris.datasource.storage.StorageAdapter; +import org.apache.doris.filesystem.FileSystem; +import org.apache.doris.filesystem.Location; +import org.apache.doris.fs.FileSystemFactory; +import org.apache.doris.service.FrontendOptions; + +import com.google.common.base.Strings; + +import java.io.File; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; + +/** + * Simple cloud plugin downloader for UDF and JDBC drivers. + */ +public class CloudPluginDownloader { + + public enum PluginType { + JDBC_DRIVERS, + JAVA_UDF, + CONNECTORS, // Reserved, not supported yet + HADOOP_CONF // Reserved, not supported yet + } + + /** + * Download plugin from cloud storage to local path + */ + public static synchronized String downloadFromCloud(PluginType type, String name, String localPath) { + validateInput(type, name); + try { + Cloud.ObjectStoreInfoPB objInfo = getCloudStorageInfo(); + String remotePath = buildS3Path(objInfo, type, name); + return doDownload(objInfo, remotePath, localPath); + } catch (Exception e) { + throw new RuntimeException("Failed to download plugin: " + e.getMessage(), e); + } + } + + /** + * Validate input parameters + */ + static void validateInput(PluginType type, String name) { + if (Strings.isNullOrEmpty(name)) { + throw new IllegalArgumentException("Plugin name cannot be empty"); + } + + if (type != PluginType.JDBC_DRIVERS && type != PluginType.JAVA_UDF) { + throw new UnsupportedOperationException("Plugin type " + type + " is not supported yet"); + } + } + + /** + * Get cloud storage info from MetaService + * Package-private for testing + */ + static Cloud.ObjectStoreInfoPB getCloudStorageInfo() throws Exception { + Cloud.GetObjStoreInfoResponse response = MetaServiceProxy.getInstance() + .getObjStoreInfo(Cloud.GetObjStoreInfoRequest.newBuilder() + .setRequestIp(FrontendOptions.getLocalHostAddressCached()) + .build()); + + if (response.getStatus().getCode() != Cloud.MetaServiceCode.OK) { + throw new RuntimeException("Failed to get storage info: " + response.getStatus().getMsg()); + } + + if (response.getObjInfoList().isEmpty()) { + throw new RuntimeException("Only SaaS cloud storage is supported currently"); + } + + return response.getObjInfo(0); + } + + /** + * Build complete S3 path from objInfo + * Package-private for testing + */ + static String buildS3Path(Cloud.ObjectStoreInfoPB objInfo, PluginType type, String name) { + String bucket = objInfo.getBucket(); + String prefix = objInfo.hasPrefix() ? objInfo.getPrefix() : ""; + String relativePath = String.format("plugins/%s/%s", type.name().toLowerCase(), name); + + String fullPath; + if (Strings.isNullOrEmpty(prefix)) { + fullPath = bucket + "/" + relativePath; + } else { + fullPath = bucket + "/" + prefix + "/" + relativePath; + } + + return "s3://" + fullPath; + } + + /** + * Execute download using SPI FileSystem + */ + private static String doDownload(Cloud.ObjectStoreInfoPB objInfo, String remotePath, String localPath) + throws Exception { + // Create parent directory + Path parentDir = Paths.get(localPath).getParent(); + if (parentDir != null && !Files.exists(parentDir)) { + Files.createDirectories(parentDir); + } + + // Delete existing file if present + File localFile = new File(localPath); + if (localFile.exists() && !localFile.delete()) { + throw new RuntimeException("Failed to delete existing file: " + localPath); + } + + // Bind the provider reported by MetaService explicitly. Raw property auto-detection is + // order-dependent when more than one storage provider recognizes the same map. + StorageAdapter storageAdapter = createStorageAdapter(objInfo); + try (FileSystem fileSystem = FileSystemFactory.getFileSystem(storageAdapter); + InputStream in = fileSystem.newInputFile(Location.of(remotePath)).newStream()) { + Files.copy(in, localFile.toPath(), StandardCopyOption.REPLACE_EXISTING); Review Comment: Do not publish the final jar pathname until the copy is complete. A mid-stream failure leaves a prefix at `localFile`; the next JDBC request sees `exists()`, skips download, and persists that prefix's MD5. During a normal copy, another reader can likewise checksum the growing file because the existence/checksum paths are outside this monitor. Copy to a unique sibling temp file, clean it on failure, then atomically move it into place after the stream closes. ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/CreateFunctionCommand.java: ########## @@ -484,6 +485,21 @@ private String getRealUrl(String url) { private String checkAndReturnDefaultJavaUdfUrl(String url) { String defaultUrl = EnvUtils.getDorisHome() + "/plugins/java_udf"; + // In cloud mode, try cloud download first + if (Config.isCloudMode()) { Review Comment: Do not intercept every cloud deployment with the legacy-SaaS downloader. A storage-vault FE may have no legacy `obj_info` at all; even if `<DORIS_HOME>/plugins/java_udf/x.jar` exists, this branch calls the downloader first and fails with `Only SaaS cloud storage is supported`, whereas the previous code loaded that local artifact. Gate this on the actual legacy object-store mode or preserve the local fallback (and cover storage-vault mode); note that the common resolver also reaches bare Python artifacts. ########## regression-test/suites/plugin_p1/test_cloud_plugin_auto_download.groovy: ########## @@ -0,0 +1,121 @@ +// 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. + +suite("test_cloud_plugin_auto_download", "p1,external") { + + //sass cloud-mode only + if (!isCloudMode() || enableStoragevault()) { + logger.info("Skip test_plugin_auto_download because not in sass cloud mode") + return + } + + String jdbcUrl = context.config.jdbcUrl + String jdbcUser = context.config.jdbcUser + String jdbcPassword = context.config.jdbcPassword + + sql """drop database if exists internal.test_auto_download_db; """ + sql """create database if not exists internal.test_auto_download_db;""" + sql """create table if not exists internal.test_auto_download_db.test_tbl + (id int, name varchar(20)) + distributed by hash(id) buckets 1 + properties('replication_num' = '1'); + """ + sql """insert into internal.test_auto_download_db.test_tbl values(1, 'auto_download_test')""" + + sql """drop catalog if exists test_auto_download_catalog """ + sql """ CREATE CATALOG `test_auto_download_catalog` PROPERTIES ( Review Comment: Establish that the JDBC jar is absent before this positive case (or use a unique uploaded object name and verify the resulting FE/BE files). With the fixed `mysql-connector-j-8.3.0.jar` name, a rerun or pre-populated image takes both FE and BE's existing-file shortcut, so the successful query can pass without testing cloud download at all. Please also use generated deterministic query output and keep final state for debugging per the regression-test rules. ########## fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java: ########## @@ -447,6 +449,18 @@ private static String checkAndReturnDefaultDriverUrl(String driverUrl) { } else if (oldTargetFile.exists()) { // File exists in old default directory return "file://" + oldTargetPath; + } else if (Config.isCloudMode()) { Review Comment: Do not let bare local existence be the permanent cache-authority decision. If the cloud key is replaced from v1 to v2, an FE retaining v1 persists the v1 MD5 while a fresh BE downloads v2 and rejects it; the inverse happens with a fresh FE and old BE, and retries never converge because neither side validates a generation. Require immutable/versioned artifact identities, or validate and atomically refresh the local cache against cloud metadata/checksum. ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/CreateFunctionCommand.java: ########## @@ -484,6 +485,21 @@ private String getRealUrl(String url) { private String checkAndReturnDefaultJavaUdfUrl(String url) { String defaultUrl = EnvUtils.getDorisHome() + "/plugins/java_udf"; + // In cloud mode, try cloud download first + if (Config.isCloudMode()) { + String targetPath = defaultUrl + "/" + url; + try { + String downloadedPath = CloudPluginDownloader.downloadFromCloud( + CloudPluginDownloader.PluginType.JAVA_UDF, url, targetPath); + if (!downloadedPath.isEmpty()) { + return "file://" + downloadedPath; + } + } catch (Exception e) { Review Comment: Preserve or log the original download failure here. This catch discards the downloader's cause, so a missing object, auth error, unavailable filesystem provider, and local copy failure all become the same generic message with no server-side trace. Match the JDBC branch by logging the exception and including a safely sanitized root-cause message (or attach the cause to the thrown exception). ########## fe/fe-core/src/main/java/org/apache/doris/catalog/JdbcResource.java: ########## @@ -447,6 +449,18 @@ private static String checkAndReturnDefaultDriverUrl(String driverUrl) { } else if (oldTargetFile.exists()) { // File exists in old default directory return "file://" + oldTargetPath; + } else if (Config.isCloudMode()) { + // Cloud mode: download from cloud to default directory + try { + String downloadedPath = CloudPluginDownloader.downloadFromCloud( + PluginType.JDBC_DRIVERS, driverUrl, targetPath); Review Comment: Include the artifact generation in the FE driver-loader identity. Even if this path refreshes v2 into the same local filename and a catalog is dropped/recreated with the v2 checksum, `JdbcConnectorClient.CLASS_LOADER_MAP` is keyed only by this URL and never evicted, so already loaded v1 driver classes remain active until FE restart. Use a checksum/versioned local URL (with safe loader retirement) and test same-key replacement plus recreate. -- 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]
