OIiveirra commented on code in PR #68585:
URL: https://github.com/apache/doris/pull/68585#discussion_r4135905562
##########
fe/fe-filesystem/fe-filesystem-hdfs-base/src/main/java/org/apache/doris/filesystem/hdfs/properties/HdfsPropertiesUtils.java:
##########
@@ -218,10 +218,14 @@ public static void checkHaConfig(Map<String, String>
hdfsProperties) {
// No nameservice configured => HA is not enabled, nothing to
validate
return;
}
- for (String dfsservice : splitAndTrim(dfsNameservices)) {
- if (dfsservice.isEmpty()) {
- continue;
- }
+ if (Arrays.stream(dfsNameservices.split(",",
-1)).anyMatch(StringUtils::isBlank)) {
Review Comment:
Addressed in commit `a96f797dec1` and covered by `HdfsPropertiesTest` and
`test_hive_ha_catalog_validation`.
Runtime HA binding now tolerates the historical trailing empty token in
`dfs.nameservices=ns1,`, while an empty nameservice list is still rejected. The
regression expectation was updated accordingly. FE/HDFS tests pass.
##########
fe/fe-core/src/main/java/org/apache/doris/fs/FileSystemFactory.java:
##########
@@ -137,14 +137,14 @@ public static
List<org.apache.doris.filesystem.properties.StorageProperties> bin
System.setProperty("doris.hadoop.config.dir",
Config.hadoop_config_dir);
FileSystemPluginManager mgr = pluginManager;
if (mgr != null) {
- return new ArrayList<>(mgr.bindAll(properties));
+ return new
ArrayList<>(mgr.bindAll(StorageAdapter.withHadoopConfigDir(properties)));
Review Comment:
Addressed in commit `a96f797dec1`.
`HdfsConfigFileLoader` now keeps absolute resource paths unchanged and only
prefixes relative paths with `_HADOOP_CONFIG_DIR_`. The HDFS unit test now
covers an absolute resource while a config directory is also configured. The
test passes.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalog.java:
##########
@@ -253,11 +256,27 @@ public boolean validatePropertiesBeforeUpdate(
} catch (IllegalArgumentException e) {
throw new DdlException(e.getMessage(), e);
}
+ validateStorageProperties(candidateProperty);
checkDriverUrlsAgainstOperatorGate(candidate, updatedProperties);
ExternalFunctionRules.check(candidateProperty.getOrDefault("function_rules",
null));
return true;
}
+ /**
+ * Bind Hive catalog storage properties during DDL validation so HDFS
configuration errors are
+ * reported by CREATE/ALTER instead of being deferred until the first
table access.
+ */
+ private void validateStorageProperties(CatalogProperty property) throws
DdlException {
+ if (!"hms".equalsIgnoreCase(getType())) {
+ return;
+ }
+ try {
+
FileSystemFactory.bindAllStorageProperties(property.getProperties());
Review Comment:
Addressed in commit `ccd6c398ee1`.
CREATE and ALTER now validate detached catalog properties outside the global
catalog write lock. Before publishing an ALTER, the write-locked path rechecks
catalog identity and the property snapshot; concurrent changes trigger a retry.
The FE build and `test_hive_ha_catalog_validation` regression pass.
--
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]
