morningman commented on code in PR #52363:
URL: https://github.com/apache/doris/pull/52363#discussion_r2189247318
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/HMSProperties.java:
##########
@@ -80,60 +112,99 @@ protected void checkRequiredProperties() {
if (!Strings.isNullOrEmpty(hiveConfResourcesConfig)) {
checkHiveConfResourcesConfig();
}
- if ("kerberos".equalsIgnoreCase(hiveMetastoreAuthenticationType)) {
- if (Strings.isNullOrEmpty(hiveMetastoreServicePrincipal)
- || Strings.isNullOrEmpty(hiveMetastoreClientPrincipal)
- || Strings.isNullOrEmpty(hiveMetastoreClientKeytab)) {
- throw new IllegalArgumentException("Hive metastore
authentication type is kerberos, "
- + "but service principal, client principal or client
keytab is not set.");
- }
- }
- if (Strings.isNullOrEmpty(hiveMetastoreUri)) {
- throw new IllegalArgumentException("Hive metastore uri is
required.");
- }
+ buildRules().validate();
}
@Override
- protected void initNormalizeAndCheckProps() {
+ public void initNormalizeAndCheckProps() {
super.initNormalizeAndCheckProps();
- hiveConfParams = loadConfigFromFile(hiveConfResourcesConfig);
- initHmsConnectionProperties();
+ initHiveConf();
+ initHadoopAuthenticator();
+ initRefreshParams();
}
- private void initHmsConnectionProperties() {
- hmsConnectionProperties = new HashMap<>();
- hmsConnectionProperties.putAll(hiveConfParams);
- hmsConnectionProperties.put("hive.metastore.authentication.type",
hiveMetastoreAuthenticationType);
- if ("kerberos".equalsIgnoreCase(hiveMetastoreAuthenticationType)) {
- hmsConnectionProperties.put("hive.metastore.service.principal",
hiveMetastoreServicePrincipal);
- hmsConnectionProperties.put("hive.metastore.client.principal",
hiveMetastoreClientPrincipal);
- hmsConnectionProperties.put("hive.metastore.client.keytab",
hiveMetastoreClientKeytab);
+ private void initUserHiveConfig(Map<String, String> origProps) {
+ if (origProps == null || origProps.isEmpty()) {
+ return;
}
- hmsConnectionProperties.put("uri", hiveMetastoreUri);
+ origProps.forEach((key, value) -> {
+ if (key.startsWith("hive.")) {
+ userOverriddenHiveConfig.put(key, value);
+ }
+ });
+ }
+
+
+ private void initRefreshParams() {
+ this.hmsEventsIncrementalSyncEnabled =
BooleanUtils.toBoolean(hmsEventsIncrementalSyncEnabledInput);
+ this.hmsEventsBatchSizePerRpc = hmsEventisBatchSizePerRpcInput;
+ }
+
+ private void initHiveConf() {
+ hiveConf = loadHiveConfFromFile(hiveConfResourcesConfig);
+ initUserHiveConfig(origProps);
+ userOverriddenHiveConfig.forEach(hiveConf::set);
+ hiveConf.set("hive.metastore.uris", hiveMetastoreUri);
+ HiveConf.setVar(hiveConf,
HiveConf.ConfVars.METASTORE_CLIENT_SOCKET_TIMEOUT,
+ String.valueOf(Config.hive_metastore_client_timeout_second));
}
private void checkHiveConfResourcesConfig() {
loadConfigFromFile(hiveConfResourcesConfig);
}
public void toPaimonOptionsAndConf(Options options) {
- hmsConnectionProperties.forEach(options::set);
+ //hmsConnectionProperties.forEach(options::set);
}
public void toIcebergHiveCatalogProperties(Map<String, String>
catalogProps) {
- hmsConnectionProperties.forEach(catalogProps::put);
+ // hmsConnectionProperties.forEach(catalogProps::put);
}
- protected Map<String, String> loadConfigFromFile(String resourceConfig) {
+ protected HiveConf loadHiveConfFromFile(String resourceConfig) {
if (Strings.isNullOrEmpty(resourceConfig)) {
- return Maps.newHashMap();
+ return new HiveConf();
+ }
+ return
CatalogConfigFileUtils.loadHiveConfFromHiveConfDir(resourceConfig);
+ }
+
+ private ParamRules buildRules() {
+
+ return new ParamRules()
+ .forbidIf(hiveMetastoreAuthenticationType, "simple", new
String[]{
+ hiveMetastoreClientPrincipal,
hiveMetastoreClientKeytab},
+ "hive.metastore.client.principal and
hive.metastore.client.keytab cannot be set when "
+ + "hive.metastore.authentication.type is
simple"
+ )
+ .requireIf(hiveMetastoreAuthenticationType, "kerberos", new
String[]{
+ hiveMetastoreClientPrincipal,
hiveMetastoreClientKeytab},
+ "hive.metastore.client.principal and
hive.metastore.client.keytab are required when "
+ + "hive.metastore.authentication.type is
kerberos");
+
+ }
+
+ private void initHadoopAuthenticator() {
Review Comment:
Add comment to explain this method
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/MetastoreProperties.java:
##########
@@ -83,99 +72,67 @@ public static Optional<Type> fromString(String input) {
}
}
- /**
- * The resolved metastore type for this configuration.
- */
@Getter
protected Type type;
- /**
- * Common property keys that may specify the metastore type.
- * These are checked in order to resolve the type from provided config.
- */
private static final List<String> POSSIBLE_TYPE_KEYS = Arrays.asList(
"metastore.type",
- "hive.metastore.type",
+ "hive.metastore.catalog.type",
"iceberg.catalog.type",
"paimon.catalog.type",
"type"
);
- /**
- * Registry mapping each {@link Type} to its constructor logic.
- */
- private static final Map<Type, Function<Map<String, String>,
MetastoreProperties>> FACTORY_MAP
- = new EnumMap<>(Type.class);
+ private static final Map<Type, MetastorePropertiesFactory> FACTORY_MAP =
new EnumMap<>(Type.class);
static {
- // Register all known factories here
- FACTORY_MAP.put(Type.HMS, HMSProperties::new);
- FACTORY_MAP.put(Type.GLUE, AWSGlueProperties::new);
- FACTORY_MAP.put(Type.DLF, AliyunDLFProperties::new);
- FACTORY_MAP.put(Type.ICEBERG_REST, IcebergRestProperties::new);
- FACTORY_MAP.put(Type.DATAPROC, DataProcProperties::new);
- FACTORY_MAP.put(Type.FILE_SYSTEM, FileMetastoreProperties::new);
+ // 注册所有一级类型
Review Comment:
English
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java:
##########
@@ -112,4 +117,18 @@ public void addProperty(String key, String val) {
public void deleteProperty(String key) {
this.properties.remove(key);
}
+
+ public MetastoreProperties getMetastoreProperties() {
Review Comment:
And better remove the `@Data` of this class.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/MetastoreProperties.java:
##########
@@ -83,99 +72,67 @@ public static Optional<Type> fromString(String input) {
}
}
- /**
- * The resolved metastore type for this configuration.
- */
@Getter
protected Type type;
- /**
- * Common property keys that may specify the metastore type.
- * These are checked in order to resolve the type from provided config.
- */
private static final List<String> POSSIBLE_TYPE_KEYS = Arrays.asList(
"metastore.type",
- "hive.metastore.type",
+ "hive.metastore.catalog.type",
Review Comment:
Why changing this?
--
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]
