CalvinKirs commented on code in PR #64695: URL: https://github.com/apache/doris/pull/64695#discussion_r3577515515
########## fe/fe-filesystem/fe-filesystem-oss-hdfs/src/main/java/org/apache/doris/filesystem/hdfs/OssHdfsFileSystemProvider.java: ########## @@ -0,0 +1,79 @@ +// 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.filesystem.hdfs; + +import org.apache.doris.filesystem.FileSystem; +import org.apache.doris.filesystem.hdfs.properties.OssHdfsProperties; +import org.apache.doris.filesystem.properties.FileSystemProperties; +import org.apache.doris.filesystem.spi.FileSystemProvider; + +import java.io.IOException; +import java.util.Locale; +import java.util.Map; + +/** + * SPI provider for Aliyun OSS-HDFS (JindoFS), addressed with {@code oss://} URIs and served by an + * HDFS-compatible {@code DFSFileSystem}. Registered via META-INF/services alongside + * {@link HdfsFileSystemProvider}. + * + * <p>Routing is kept strictly disjoint from plain HDFS: the authoritative {@code _STORAGE_TYPE_} + * marker set by fe-core's StoragePropertiesConverter wins, and the heuristic fallback keys on + * {@link OssHdfsProperties#guessIsMe} (oss.hdfs flag / oss-dls endpoint) or an {@code oss://} URI. + * The framework selects providers first-match-wins over an unordered list, so the two providers' + * {@code supports()} predicates must not overlap.</p> + */ +public class OssHdfsFileSystemProvider implements FileSystemProvider<FileSystemProperties> { + + @Override + public boolean supports(Map<String, String> properties) { + String storageType = properties.get("_STORAGE_TYPE_"); + if ("OSS_HDFS".equals(storageType)) { + return true; + } + if ("HDFS".equals(storageType)) { + // An explicit plain-HDFS marker belongs to HdfsFileSystemProvider, never here. + return false; + } + if ("OSS".equals(storageType)) { + // A native-OSS (S3-compatible) marker belongs to OssFileSystemProvider, never here. + return false; + } + // No authoritative marker: fall back to the same detection fe-core uses to pick + // OSSHdfsProperties, plus a bare oss:// URI. + if (OssHdfsProperties.guessIsMe(properties)) { + return true; + } + String uri = properties.get("HDFS_URI"); + if (uri == null) { + uri = properties.get("fs.defaultFS"); + } + return uri != null && uri.toLowerCase(Locale.ROOT).startsWith("oss://"); Review Comment: Fixed in 54920958012. Both providers now mirror `StorageProperties.createPrimary` precedence: `OssHdfsFileSystemProvider` claims `fs.oss-hdfs.support=true` / `oss.hdfs.enabled=true` (they outrank everything below, including `fs.oss.support`), and yields when `fs.oss.support=true` so the bare `oss://` URI fallback cannot steal an explicitly native-OSS map; `OssFileSystemProvider` declines the two OSS-HDFS flags before its explicit/endpoint checks. Unit tests cover both scenarios from this comment plus the both-flags precedence case. ########## fe/fe-filesystem/fe-filesystem-s3/src/main/java/org/apache/doris/filesystem/s3/S3FileSystemProperties.java: ########## @@ -213,6 +214,11 @@ public void validate() { .validate("Invalid S3 filesystem properties"); } + @Override + public Set<String> getSupportedSchemes() { Review Comment: Half right — the TVF example doesn't apply, but the class of issue was real elsewhere; fixed in 0f5f7c1b622 without touching the whitelist. `S3TableValuedFunction` normalizes at analysis time (`filePath = storageProperties.validateAndNormalizeUri(uri)`), so `globListWithLimit` already receives `s3://` — the existing cos/cosn TVF regression cases keep working. Broker load (`LoadCommand` via `BrokerDesc.getFileLocation`) and `parquet_meta` normalize too. The callers that really passed raw user URIs to a concrete filesystem were OUTFILE/EXPORT `delete_existing_files` and the restv2 import preview endpoint; those now normalize through `BrokerDesc.getFileLocation` before crossing the plugin boundary, consistent with the rule that concrete filesystems only ever see their native schemes. ########## fe/fe-filesystem/fe-filesystem-oss-hdfs/src/main/java/org/apache/doris/filesystem/hdfs/properties/OssHdfsProperties.java: ########## @@ -0,0 +1,167 @@ +// 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.filesystem.hdfs.properties; + +import org.apache.doris.foundation.property.ConnectorProperty; + +import com.google.common.collect.ImmutableSet; +import org.apache.commons.lang3.StringUtils; + +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Aliyun OSS-HDFS (JindoFS) storage properties for fe-filesystem. + * + * <p>OSS-HDFS is an HDFS-compatible service over OSS, addressed with {@code oss://} URIs and + * served by the JindoFS Hadoop FileSystem ({@code com.aliyun.jindodata.oss.JindoOssFileSystem}). + * It therefore sits on the shared {@link HdfsCompatibleProperties} base alongside plain + * {@link HdfsProperties} and produces a backend config map for {@code DFSFileSystem}; the only + * delta is the Jindo {@code fs.oss.*} wiring plus oss-dls endpoint/region handling.</p> + * + * <p>Self-contained port of the kernel {@code OSSHdfsProperties}, with zero fe-core / fe-common + * dependency. The Jindo impl class names are kept as string constants (loaded reflectively by the + * backend), so no compile dependency on the Jindo SDK is introduced.</p> + */ +public class OssHdfsProperties extends HdfsCompatibleProperties { + + private static final String JINDO_OSS_FILE_SYSTEM_IMPL = + "com.aliyun.jindodata.oss.JindoOssFileSystem"; + private static final String JINDO_OSS_ABSTRACT_FILE_SYSTEM_IMPL = + "com.aliyun.jindodata.oss.JindoOSS"; + + private static final String OSS_HDFS_PREFIX_KEY = "oss.hdfs."; + private static final String OSS_HDFS_ENDPOINT_SUFFIX = ".oss-dls.aliyuncs.com"; + + private static final Set<String> OSS_ENDPOINT_KEY_NAME = ImmutableSet.of("oss.hdfs.endpoint", + "oss.endpoint", "dlf.endpoint", "dlf.catalog.endpoint"); + + private static final Set<Pattern> ENDPOINT_PATTERN = ImmutableSet.of( + Pattern.compile("(?:https?://)?([a-z]{2}-[a-z0-9-]+)\\.oss-dls\\.aliyuncs\\.com"), + Pattern.compile("^(?:https?://)?dlf(?:-vpc)?\\.([a-z0-9-]+)\\.aliyuncs\\.com(?:/.*)?$")); + + private static final Set<String> SUPPORT_SCHEMA = ImmutableSet.of("oss", "hdfs"); + + @ConnectorProperty(names = {"oss.hdfs.endpoint", "oss.endpoint", "dlf.endpoint", "dlf.catalog.endpoint"}, Review Comment: Fixed in 54920958012. The plugin's `OssHdfsProperties` now accepts the normalized backend keys as aliases (`fs.oss.endpoint`, `fs.oss.accessKeyId`, `fs.oss.accessKeySecret`, `fs.oss.region`, `fs.oss.securityToken`), following the fe-filesystem-oss precedent, so the binder is a fixpoint for `StoragePropertiesConverter.toMap()` output and `create()` succeeds on converted maps. Added round-trip unit tests binding exactly the converter-shaped map. -- 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]
