This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.0 by this push:
new 698b28ac319 branch-4.0: [Fix](Iam-role)Ensure StorageProperties list
remains ordered when auto-loading default HDFS #58968 (#59072)
698b28ac319 is described below
commit 698b28ac31904d47a11127958af1f0fa6bac8c20
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Wed Dec 17 16:06:20 2025 +0800
branch-4.0: [Fix](Iam-role)Ensure StorageProperties list remains ordered
when auto-loading default HDFS #58968 (#59072)
Cherry-picked from #58968
Co-authored-by: Calvin Kirs <[email protected]>
---
.../apache/doris/datasource/CatalogProperty.java | 30 +++++++++++++++++++---
.../datasource/iceberg/IcebergExternalCatalog.java | 5 ++--
.../datasource/paimon/PaimonExternalCatalog.java | 2 +-
.../property/storage/StorageProperties.java | 23 +++++++++++------
.../property/storage/AzurePropertiesTest.java | 4 +--
.../property/storage/COSPropertiesTest.java | 7 ++---
.../property/storage/OBSPropertyTest.java | 8 +++---
.../property/storage/OSSPropertiesTest.java | 7 ++---
.../property/storage/S3PropertiesTest.java | 3 ++-
9 files changed, 62 insertions(+), 27 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
index 6e5d5281796..0ad7b66c39b 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
@@ -53,6 +53,21 @@ public class CatalogProperty {
@SerializedName(value = "properties")
private Map<String, String> properties;
+ /**
+ * An ordered list of all initialized {@link StorageProperties} instances.
+ * <p>
+ * The order of this list is significant:
+ * <ul>
+ * <li>The default HDFSProperties (if auto-created) is always inserted
at index 0.</li>
+ * <li>Explicitly configured storage providers follow in the order they
are detected.</li>
+ * <li>Callers rely on this deterministic ordering for selecting or
iterating through
+ * storage backends.</li>
+ * </ul>
+ * <p>
+ * Declared as {@code volatile} to ensure visibility across threads once
initialized.
+ */
+ private volatile List<StorageProperties> orderedStoragePropertiesList;
+
// Lazy-loaded storage properties map, using volatile to ensure visibility
private volatile Map<StorageProperties.Type, StorageProperties>
storagePropertiesMap;
@@ -137,13 +152,13 @@ public class CatalogProperty {
/**
* Get storage properties map with lazy loading, using double-check
locking to ensure thread safety
*/
- public Map<StorageProperties.Type, StorageProperties>
getStoragePropertiesMap() {
+ private void initStorageProperties() {
if (storagePropertiesMap == null) {
synchronized (this) {
if (storagePropertiesMap == null) {
try {
- List<StorageProperties> storageProperties =
StorageProperties.createAll(getProperties());
- this.storagePropertiesMap = storageProperties.stream()
+ this.orderedStoragePropertiesList =
StorageProperties.createAll(getProperties());
+ this.storagePropertiesMap =
orderedStoragePropertiesList.stream()
.collect(Collectors.toMap(StorageProperties::getType, Function.identity()));
} catch (UserException e) {
LOG.warn("Failed to initialize catalog storage
properties", e);
@@ -153,9 +168,18 @@ public class CatalogProperty {
}
}
}
+ }
+
+ public Map<StorageProperties.Type, StorageProperties>
getStoragePropertiesMap() {
+ initStorageProperties();
return storagePropertiesMap;
}
+ public List<StorageProperties> getOrderedStoragePropertiesList() {
+ initStorageProperties();
+ return orderedStoragePropertiesList;
+ }
+
public void checkMetaStoreAndStorageProperties(Class msClass) {
MetastoreProperties msProperties;
List<StorageProperties> storageProperties;
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalCatalog.java
index 02d24cc0146..90d7ab41ebe 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalCatalog.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergExternalCatalog.java
@@ -34,7 +34,6 @@ import org.apache.doris.transaction.TransactionManagerFactory;
import org.apache.iceberg.catalog.Catalog;
-import java.util.ArrayList;
import java.util.List;
public abstract class IcebergExternalCatalog extends ExternalCatalog {
@@ -60,8 +59,8 @@ public abstract class IcebergExternalCatalog extends
ExternalCatalog {
protected void initCatalog() {
try {
msProperties = (AbstractIcebergProperties)
catalogProperty.getMetastoreProperties();
- this.catalog = msProperties.initializeCatalog(getName(), new
ArrayList<>(catalogProperty
- .getStoragePropertiesMap().values()));
+ this.catalog = msProperties.initializeCatalog(getName(),
catalogProperty
+ .getOrderedStoragePropertiesList());
this.icebergCatalogType = msProperties.getIcebergCatalogType();
} catch (ClassCastException e) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalog.java
index 76e093656bb..09ec08e904d 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalog.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonExternalCatalog.java
@@ -173,7 +173,7 @@ public class PaimonExternalCatalog extends ExternalCatalog {
protected Catalog createCatalog() {
try {
return paimonProperties.initializeCatalog(getName(), new
ArrayList<>(catalogProperty
- .getStoragePropertiesMap().values()));
+ .getOrderedStoragePropertiesList()));
} catch (Exception e) {
throw new RuntimeException("Failed to create catalog, catalog
name: " + getName() + ", exception: "
+ ExceptionUtils.getRootCauseMessage(e), e);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/StorageProperties.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/StorageProperties.java
index 3e8a703204b..5de749f45d6 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/StorageProperties.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/StorageProperties.java
@@ -56,7 +56,7 @@ public abstract class StorageProperties extends
ConnectionProperties {
public static final String FS_PROVIDER_KEY = "provider";
- protected final String userFsPropsPrefix = "fs.";
+ protected final String userFsPropsPrefix = "fs.";
public enum Type {
HDFS,
@@ -118,12 +118,19 @@ public abstract class StorageProperties extends
ConnectionProperties {
/**
* Creates a list of StorageProperties instances based on the provided
properties.
* <p>
- * This method iterates through the list of supported storage types and
creates an instance
- * for each supported type. If no supported type is found, an
HDFSProperties instance is added
- * by default.
+ * This method iterates through all registered storage providers and
constructs one
+ * {@link StorageProperties} instance for each provider that recognizes
the given properties.
+ * <p>
+ * If no HDFSProperties is explicitly configured, a default HDFSProperties
will be added
+ * automatically. The default HDFSProperties is inserted at index 0 to
ensure that:
+ * <ul>
+ * <li>The list preserves a deterministic order (it is an ordered
List).</li>
+ * <li>The default HDFS configuration does not override or shadow
explicitly configured
+ * object storage providers, which are appended after detection.</li>
+ * </ul>
*
- * @param origProps the original properties map to create the
StorageProperties instances
- * @return a list of StorageProperties instances for all supported storage
types
+ * @param origProps the raw property map used to initialize each
StorageProperties instance
+ * @return an ordered list of StorageProperties instances
*/
public static List<StorageProperties> createAll(Map<String, String>
origProps) throws UserException {
List<StorageProperties> result = new ArrayList<>();
@@ -135,7 +142,7 @@ public abstract class StorageProperties extends
ConnectionProperties {
}
// Add default HDFS storage if not explicitly configured
if (result.stream().noneMatch(HdfsProperties.class::isInstance)) {
- result.add(new HdfsProperties(origProps, false));
+ result.add(0, new HdfsProperties(origProps, false));
}
for (StorageProperties storageProperties : result) {
@@ -203,7 +210,7 @@ public abstract class StorageProperties extends
ConnectionProperties {
|| LocalProperties.guessIsMe(props)) ? new
LocalProperties(props) : null,
props -> (isFsSupport(props, FS_HTTP_SUPPORT)
|| HttpProperties.guessIsMe(props)) ? new
HttpProperties(props) : null
- );
+ );
protected StorageProperties(Type type, Map<String, String> origProps) {
super(origProps);
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/AzurePropertiesTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/AzurePropertiesTest.java
index 927a9f1c148..8171f33db40 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/AzurePropertiesTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/AzurePropertiesTest.java
@@ -80,8 +80,8 @@ public class AzurePropertiesTest {
origProps.put("s3.secret_key", "myAzureSecretKey");
List<StorageProperties> storagePropertiesList =
StorageProperties.createAll(origProps);
Assertions.assertEquals(2, storagePropertiesList.size());
- Assertions.assertEquals(HdfsProperties.class,
storagePropertiesList.get(1).getClass());
- Assertions.assertEquals(AzureProperties.class,
storagePropertiesList.get(0).getClass());
+ Assertions.assertEquals(HdfsProperties.class,
storagePropertiesList.get(0).getClass());
+ Assertions.assertEquals(AzureProperties.class,
storagePropertiesList.get(1).getClass());
origProps.put("s3.endpoint", "https://mystorageaccount.net";);
// Expect an exception due to missing provider
origProps.put("provider", "azure");
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/COSPropertiesTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/COSPropertiesTest.java
index df20ea5d333..d4b0277c946 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/COSPropertiesTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/COSPropertiesTest.java
@@ -56,7 +56,7 @@ public class COSPropertiesTest {
origProps.put("test_non_storage_param", "6000");
Assertions.assertDoesNotThrow(() ->
StorageProperties.createAll(origProps));
origProps.put("cos.endpoint", "cos.ap-beijing-1.myqcloud.com");
- COSProperties cosProperties = (COSProperties)
StorageProperties.createAll(origProps).get(0);
+ COSProperties cosProperties = (COSProperties)
StorageProperties.createAll(origProps).get(1);
Map<String, String> cosConfig = cosProperties.getMatchedProperties();
Assertions.assertTrue(!cosConfig.containsKey("test_non_storage_param"));
@@ -90,7 +90,7 @@ public class COSPropertiesTest {
origProps.put(StorageProperties.FS_COS_SUPPORT, "true");
//origProps.put("cos.region", "ap-beijing");
- COSProperties cosProperties = (COSProperties)
StorageProperties.createAll(origProps).get(0);
+ COSProperties cosProperties = (COSProperties)
StorageProperties.createAll(origProps).get(1);
Map<String, String> s3Props =
cosProperties.generateBackendS3Configuration();
Map<String, String> cosConfig = cosProperties.getMatchedProperties();
Assertions.assertTrue(!cosConfig.containsKey("test_non_storage_param"));
@@ -110,7 +110,8 @@ public class COSPropertiesTest {
Assertions.assertEquals("1000",
s3Props.get("AWS_CONNECTION_TIMEOUT_MS"));
Assertions.assertEquals("false", s3Props.get("use_path_style"));
origProps.put("cos.use_path_style", "true");
- cosProperties = (COSProperties)
StorageProperties.createAll(origProps).get(0);
+ cosProperties = (COSProperties)
StorageProperties.createAll(origProps).get(1);
+ Assertions.assertEquals(HdfsProperties.class,
StorageProperties.createAll(origProps).get(0).getClass());
s3Props = cosProperties.generateBackendS3Configuration();
Assertions.assertEquals("true", s3Props.get("use_path_style"));
// Add any additional assertions for other properties if needed
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OBSPropertyTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OBSPropertyTest.java
index 4329368254a..06c2b36cf7d 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OBSPropertyTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OBSPropertyTest.java
@@ -70,7 +70,8 @@ public class OBSPropertyTest {
origProps.put("obs.use_path_style", "true");
origProps.put("test_non_storage_param", "test_non_storage_value");
origProps.put(StorageProperties.FS_OBS_SUPPORT, "true");
- OBSProperties obsProperties = (OBSProperties)
StorageProperties.createAll(origProps).get(0);
+ OBSProperties obsProperties = (OBSProperties)
StorageProperties.createAll(origProps).get(1);
+ Assertions.assertEquals(HdfsProperties.class,
StorageProperties.createAll(origProps).get(0).getClass());
Map<String, String> s3Props = new HashMap<>();
Map<String, String> obsConfig = obsProperties.getMatchedProperties();
Assertions.assertTrue(!obsConfig.containsKey("test_non_storage_param"));
@@ -91,7 +92,7 @@ public class OBSPropertyTest {
Assertions.assertEquals("1000",
s3Props.get("AWS_CONNECTION_TIMEOUT_MS"));
Assertions.assertEquals("true", s3Props.get("use_path_style"));
origProps.remove("obs.use_path_style");
- obsProperties = (OBSProperties)
StorageProperties.createAll(origProps).get(0);
+ obsProperties = (OBSProperties)
StorageProperties.createAll(origProps).get(1);
s3Props = obsProperties.getBackendConfigProperties();
Assertions.assertEquals("false", s3Props.get("use_path_style"));
}
@@ -102,7 +103,8 @@ public class OBSPropertyTest {
origProps.put("obs.endpoint", "obs.cn-north-4.myhuaweicloud.com");
origProps.put("obs.access_key", "myOBSAccessKey");
origProps.put("obs.secret_key", "myOBSSecretKey");
- OBSProperties obsProperties = (OBSProperties)
StorageProperties.createAll(origProps).get(0);
+ OBSProperties obsProperties = (OBSProperties)
StorageProperties.createAll(origProps).get(1);
+ Assertions.assertEquals(HdfsProperties.class,
StorageProperties.createAll(origProps).get(0).getClass());
Assertions.assertEquals("cn-north-4", obsProperties.getRegion());
Assertions.assertEquals("myOBSAccessKey",
obsProperties.getAccessKey());
Assertions.assertEquals("myOBSSecretKey",
obsProperties.getSecretKey());
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OSSPropertiesTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OSSPropertiesTest.java
index 0bc5e823f0e..dbf34751742 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OSSPropertiesTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OSSPropertiesTest.java
@@ -70,7 +70,8 @@ public class OSSPropertiesTest {
origProps.put("oss.connection.timeout", "1000");
origProps.put("oss.use_path_style", "true");
origProps.put("test_non_storage_param", "6000");
- OSSProperties ossProperties = (OSSProperties)
StorageProperties.createAll(origProps).get(0);
+ OSSProperties ossProperties = (OSSProperties)
StorageProperties.createAll(origProps).get(1);
+ Assertions.assertEquals(HdfsProperties.class,
StorageProperties.createAll(origProps).get(0).getClass());
Map<String, String> s3Props;
Map<String, String> ossConfig = ossProperties.getMatchedProperties();
@@ -93,7 +94,7 @@ public class OSSPropertiesTest {
Assertions.assertEquals("1000",
s3Props.get("AWS_CONNECTION_TIMEOUT_MS"));
Assertions.assertEquals("true", s3Props.get("use_path_style"));
origProps.remove("oss.use_path_style");
- ossProperties = (OSSProperties)
StorageProperties.createAll(origProps).get(0);
+ ossProperties = (OSSProperties)
StorageProperties.createAll(origProps).get(1);
s3Props = ossProperties.generateBackendS3Configuration();
Assertions.assertEquals("false", s3Props.get("use_path_style"));
}
@@ -120,7 +121,7 @@ public class OSSPropertiesTest {
origProps.put("oss.endpoint",
"http://s3.oss-cn-hongkong.aliyuncs.com";);
Assertions.assertEquals("cn-hongkong", ((OSSProperties)
StorageProperties.createPrimary(origProps)).getRegion());
origProps.put("oss.endpoint", "https://dlf.cn-beijing.aliyuncs.com";);
- Assertions.assertEquals("cn-beijing", ((OSSProperties)
StorageProperties.createAll(origProps).get(0)).getRegion());
+ Assertions.assertEquals("cn-beijing", ((OSSProperties)
StorageProperties.createAll(origProps).get(1)).getRegion());
origProps.put("oss.endpoint", "datalake-vpc.cn-shenzhen.aliyuncs.com");
Assertions.assertEquals("cn-shenzhen", ((OSSProperties)
StorageProperties.createPrimary(origProps)).getRegion());
origProps.put("oss.endpoint",
"https://datalake-vpc.cn-shenzhen.aliyuncs.com";);
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/S3PropertiesTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/S3PropertiesTest.java
index f4ea2fcb381..0c279df3368 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/S3PropertiesTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/S3PropertiesTest.java
@@ -130,7 +130,8 @@ public class S3PropertiesTest {
origProps.put("s3.connection.timeout", "6000");
origProps.put("test_non_storage_param", "6000");
origProps.put("s3.endpoint", "s3.us-west-1.amazonaws.com");
- S3Properties s3Properties = (S3Properties)
StorageProperties.createAll(origProps).get(0);
+ S3Properties s3Properties = (S3Properties)
StorageProperties.createAll(origProps).get(1);
+ Assertions.assertEquals(HdfsProperties.class,
StorageProperties.createAll(origProps).get(0).getClass());
Map<String, String> s3Props =
s3Properties.getBackendConfigProperties();
Map<String, String> s3Config = s3Properties.getMatchedProperties();
Assertions.assertTrue(!s3Config.containsKey("test_non_storage_param"));
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
