github-actions[bot] commented on code in PR #67545:
URL: https://github.com/apache/doris/pull/67545#discussion_r3947049711
##########
fe/fe-filesystem/fe-filesystem-oss-hdfs/src/main/java/org/apache/doris/filesystem/hdfs/properties/OssHdfsProperties.java:
##########
@@ -137,7 +142,30 @@ public String validateAndNormalizeUri(String uri) {
if (!uriObj.getScheme().equalsIgnoreCase("oss")) {
throw new IllegalArgumentException("The uri scheme is not oss.");
}
- return uriObj.toString();
+ String authority = uriObj.getRawAuthority();
+ if (StringUtils.isBlank(authority)
+ ||
authority.toLowerCase(Locale.ROOT).endsWith(OSS_HDFS_ENDPOINT_SUFFIX)) {
+ return uriObj.toString();
Review Comment:
[P2] Preserve OSS-HDFS identity for mixed-case qualified paths
This recognizes a mixed-case `.OSS-DLS.ALIYUNCS.COM` authority as already
normalized, but returns its original case. The new fallback therefore selects
the OSS-HDFS adapter successfully, while the later
`LocationPath.isHdfsOnOssEndpoint()` checks remain case-sensitive and classify
the normalized path as `FILE_S3`. `PluginDrivenSplit` then sends data/delete
ranges down the S3 path, where BE rejects the preserved `oss://` URI;
`getBackendFileType()` gives writes the same wrong type. This is distinct from
the existing mixed-case DLF configuration-endpoint thread: the binding is
correct here and only an already-qualified data URI loses its identity at
dispatch. Please canonicalize the authority or make the downstream marker
checks case-insensitive, and cover mixed-case qualified data/deletion-vector
paths plus BE file-type selection.
##########
fe/fe-filesystem/fe-filesystem-oss-hdfs/src/main/java/org/apache/doris/filesystem/hdfs/properties/OssHdfsProperties.java:
##########
@@ -137,7 +142,30 @@ public String validateAndNormalizeUri(String uri) {
if (!uriObj.getScheme().equalsIgnoreCase("oss")) {
throw new IllegalArgumentException("The uri scheme is not oss.");
}
- return uriObj.toString();
+ String authority = uriObj.getRawAuthority();
+ if (StringUtils.isBlank(authority)
+ ||
authority.toLowerCase(Locale.ROOT).endsWith(OSS_HDFS_ENDPOINT_SUFFIX)) {
+ return uriObj.toString();
+ }
+
+ // The connector SPI carries only the normalized URI, not the selected
adapter. Embedding
+ // the configured endpoint keeps plain bucket paths identifiable as
Jindo/HDFS downstream.
+ String endpointHost = extractEndpointHost(endpoint);
+ String normalizedUri = uriObj.toString();
+ int authorityStart = uriObj.getScheme().length() + 3;
+ int authorityEnd = authorityStart + authority.length();
+ return normalizedUri.substring(0, authorityStart)
+ + authority + "." + endpointHost
+ + normalizedUri.substring(authorityEnd);
Review Comment:
[P1] Replace the native OSS endpoint instead of appending it
With only the OSS-HDFS adapter bound, the new fallback deliberately routes
every `oss://` path through this normalizer. For the already supported
virtual-hosted form `oss://b.oss-cn-beijing.aliyuncs.com/k`, this concatenation
produces
`oss://b.oss-cn-beijing.aliyuncs.com.cn-beijing.oss-dls.aliyuncs.com/k`, so
Jindo treats the old endpoint labels as part of the bucket and cannot address
the intended object. The same path is used for Iceberg/Paimon data and delete
files, Iceberg writes, and the Paimon BE connectivity probe. This is distinct
from the existing plain-URI adapter-selection thread: selection succeeds here,
but an already native-OSS-qualified URI is corrupted. Please isolate the bucket
and replace the native OSS endpoint with the configured DLS endpoint, with
coverage for qualified data/delete/write and connectivity paths.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java:
##########
@@ -503,7 +516,12 @@ ConnectorTestResult probeStorageFromBackend(String
location) {
* (Iceberg {@code warehouse} or Polaris {@code default-base-location}).
*/
private String resolveS3TestLocation(String catalogType) {
- String location =
toS3Location(properties.get(CatalogProperties.WAREHOUSE_LOCATION));
+ String warehouse =
properties.get(CatalogProperties.WAREHOUSE_LOCATION);
+ if (IcebergCatalogProperties.TYPE_DLF.equalsIgnoreCase(catalogType)
+ && warehouse != null &&
warehouse.trim().toLowerCase(Locale.ROOT).startsWith("oss://")) {
+ warehouse = "s3://" +
warehouse.trim().substring("oss://".length());
+ }
Review Comment:
[P1] Normalize the qualified OSS warehouse before probing
For the supported native-OSS form
`warehouse=oss://b.oss-cn-hangzhou.aliyuncs.com/wh`, this only changes the
scheme and probes `s3://b.oss-cn-hangzhou.aliyuncs.com/wh`. Both Iceberg
`S3FileIO` and the BE probe then treat that entire authority as the bucket,
whereas the normal OSS runtime binding canonicalizes the same URI to
`s3://b/wh`; with the separately configured OSS endpoint and default
virtual-host mode, the probe reaches a different bucket/host and can reject an
otherwise valid catalog. This is distinct from the earlier missing DLF probe
thread: the probe now runs, but targets the wrong bucket for an accepted
warehouse shape. Please normalize the warehouse through the selected OSS
binding (or strip its native endpoint suffix) before both probes, and add a
connectivity test for an endpoint-qualified OSS warehouse.
--
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]
