This is an automated email from the ASF dual-hosted git repository.
rexxiong pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/celeborn.git
The following commit(s) were added to refs/heads/main by this push:
new 9fefa6631 [CELEBORN-1550][FOLLOWUP] Support
celeborn.dynamicConfig.store.backend with short name and backend implementation
9fefa6631 is described below
commit 9fefa66318e5c6e0f2237f63200ade8aba367e75
Author: SteNicholas <[email protected]>
AuthorDate: Fri Aug 23 13:53:21 2024 +0800
[CELEBORN-1550][FOLLOWUP] Support celeborn.dynamicConfig.store.backend with
short name and backend implementation
### What changes were proposed in this pull request?
Introduce `ConfigStore` to support `celeborn.dynamicConfig.store.backend`
with short name and backend implementation.
### Why are the changes needed?
`celeborn.dynamicConfig.store.backend` is allowed to be specified in two
ways:
- Using short names: Default available options are FS, DB.
- Using the fully qualified class name of the backend implementation.
Therefore, it's recommended to introduce `ConfigStore` based on SPI
mechanism for `celeborn.dynamicConfig.store.backend` instead of
`dynamicConfigStoreBackendShortNames` which could not add other short name of
backend implementation for users.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
CI.
Closes #2698 from SteNicholas/CELEBORN-1550.
Authored-by: SteNicholas <[email protected]>
Signed-off-by: Shuang <[email protected]>
---
.../org/apache/celeborn/common/CelebornConf.scala | 10 +++---
docs/configuration/master.md | 2 +-
docs/configuration/worker.md | 2 +-
.../common/service/config/ConfigService.java | 7 ----
.../server/common/service/config/ConfigStore.java | 39 ++++++++++++++++++++++
.../common/service/config/DbConfigServiceImpl.java | 5 ---
.../common/service/config/DbConfigStoreImpl.java | 31 +++++++++++++++++
.../config/DynamicConfigServiceFactory.java | 30 ++++++-----------
.../common/service/config/FsConfigServiceImpl.java | 5 ---
.../common/service/config/FsConfigStoreImpl.java | 31 +++++++++++++++++
...leborn.server.common.service.config.ConfigStore | 19 +++++++++++
11 files changed, 138 insertions(+), 43 deletions(-)
diff --git
a/common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
b/common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
index 1bbb93f28..53ac17398 100644
--- a/common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
+++ b/common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
@@ -5262,10 +5262,12 @@ object CelebornConf extends Logging {
val DYNAMIC_CONFIG_STORE_BACKEND: OptionalConfigEntry[String] =
buildConf("celeborn.dynamicConfig.store.backend")
.categories("master", "worker")
- .doc("Store backend for dynamic config service. The backend can be
specified in two ways:" +
- " - Using short names: Default available options are FS, DB." +
- " - Using the fully qualified class name of the backend
implementation." +
- "If not provided, it means that dynamic configuration is disabled.")
+ .doc(
+ "Store backend for dynamic config service. The store backend can be
specified in two ways:" +
+ " - Using the short name of the store backend defined in the
implementation of `ConfigStore#getName` " +
+ "whose return value can be mapped to the corresponding backend
implementation. Available options: FS, DB." +
+ " - Using the service class name of the store backend
implementation." +
+ "If not provided, it means that dynamic configuration is disabled.")
.version("0.4.0")
.stringConf
.createOptional
diff --git a/docs/configuration/master.md b/docs/configuration/master.md
index 7b7b3c646..11d1ff8fc 100644
--- a/docs/configuration/master.md
+++ b/docs/configuration/master.md
@@ -21,7 +21,7 @@ license: |
| --- | ------- | --------- | ----------- | ----- | ---------- |
| celeborn.cluster.name | default | false | Celeborn cluster name. | 0.5.0 |
|
| celeborn.dynamicConfig.refresh.interval | 120s | false | Interval for
refreshing the corresponding dynamic config periodically. | 0.4.0 | |
-| celeborn.dynamicConfig.store.backend | <undefined> | false | Store
backend for dynamic config service. The backend can be specified in two ways: -
Using short names: Default available options are FS, DB. - Using the fully
qualified class name of the backend implementation.If not provided, it means
that dynamic configuration is disabled. | 0.4.0 | |
+| celeborn.dynamicConfig.store.backend | <undefined> | false | Store
backend for dynamic config service. The store backend can be specified in two
ways: - Using the short name of the store backend defined in the implementation
of `ConfigStore#getName` whose return value can be mapped to the corresponding
backend implementation. Available options: FS, DB. - Using the service class
name of the store backend implementation.If not provided, it means that dynamic
configuration is disabl [...]
| celeborn.dynamicConfig.store.db.fetch.pageSize | 1000 | false | The page
size for db store to query configurations. | 0.5.0 | |
| celeborn.dynamicConfig.store.db.hikari.connectionTimeout | 30s | false | The
connection timeout that a client will wait for a connection from the pool for
db store backend. | 0.5.0 | |
| celeborn.dynamicConfig.store.db.hikari.driverClassName | | false | The jdbc
driver class name of db store backend. | 0.5.0 | |
diff --git a/docs/configuration/worker.md b/docs/configuration/worker.md
index 75f861feb..7a1b2361a 100644
--- a/docs/configuration/worker.md
+++ b/docs/configuration/worker.md
@@ -21,7 +21,7 @@ license: |
| --- | ------- | --------- | ----------- | ----- | ---------- |
| celeborn.cluster.name | default | false | Celeborn cluster name. | 0.5.0 |
|
| celeborn.dynamicConfig.refresh.interval | 120s | false | Interval for
refreshing the corresponding dynamic config periodically. | 0.4.0 | |
-| celeborn.dynamicConfig.store.backend | <undefined> | false | Store
backend for dynamic config service. The backend can be specified in two ways: -
Using short names: Default available options are FS, DB. - Using the fully
qualified class name of the backend implementation.If not provided, it means
that dynamic configuration is disabled. | 0.4.0 | |
+| celeborn.dynamicConfig.store.backend | <undefined> | false | Store
backend for dynamic config service. The store backend can be specified in two
ways: - Using the short name of the store backend defined in the implementation
of `ConfigStore#getName` whose return value can be mapped to the corresponding
backend implementation. Available options: FS, DB. - Using the service class
name of the store backend implementation.If not provided, it means that dynamic
configuration is disabl [...]
| celeborn.dynamicConfig.store.db.fetch.pageSize | 1000 | false | The page
size for db store to query configurations. | 0.5.0 | |
| celeborn.dynamicConfig.store.db.hikari.connectionTimeout | 30s | false | The
connection timeout that a client will wait for a connection from the pool for
db store backend. | 0.5.0 | |
| celeborn.dynamicConfig.store.db.hikari.driverClassName | | false | The jdbc
driver class name of db store backend. | 0.5.0 | |
diff --git
a/service/src/main/java/org/apache/celeborn/server/common/service/config/ConfigService.java
b/service/src/main/java/org/apache/celeborn/server/common/service/config/ConfigService.java
index 94e3d6dd1..1d8531654 100644
---
a/service/src/main/java/org/apache/celeborn/server/common/service/config/ConfigService.java
+++
b/service/src/main/java/org/apache/celeborn/server/common/service/config/ConfigService.java
@@ -110,11 +110,4 @@ public interface ConfigService {
/** Shutdowns configuration management service. */
void shutdown();
-
- /**
- * Retrieves the name of the configuration service.
- *
- * @return The name of the configuration service
- */
- String getName();
}
diff --git
a/service/src/main/java/org/apache/celeborn/server/common/service/config/ConfigStore.java
b/service/src/main/java/org/apache/celeborn/server/common/service/config/ConfigStore.java
new file mode 100644
index 000000000..eeda24cc1
--- /dev/null
+++
b/service/src/main/java/org/apache/celeborn/server/common/service/config/ConfigStore.java
@@ -0,0 +1,39 @@
+/*
+ * 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.celeborn.server.common.service.config;
+
+/**
+ * Config store defines store backend for dynamic config service including
short name and config
+ * service implementation.
+ */
+public interface ConfigStore {
+
+ /**
+ * Gets short name of store backend for dynamic config service.
+ *
+ * @return The short name of store backend for dynamic config service.
+ */
+ String getName();
+
+ /**
+ * Gets service class name of store backend for dynamic config service.
+ *
+ * @return The service class name of store backend for dynamic config
service.
+ */
+ String getService();
+}
diff --git
a/service/src/main/java/org/apache/celeborn/server/common/service/config/DbConfigServiceImpl.java
b/service/src/main/java/org/apache/celeborn/server/common/service/config/DbConfigServiceImpl.java
index 993b669df..7ef0fcd5f 100644
---
a/service/src/main/java/org/apache/celeborn/server/common/service/config/DbConfigServiceImpl.java
+++
b/service/src/main/java/org/apache/celeborn/server/common/service/config/DbConfigServiceImpl.java
@@ -57,11 +57,6 @@ public class DbConfigServiceImpl extends
BaseConfigServiceImpl implements Config
Function.identity())));
}
- @Override
- public String getName() {
- return "DB";
- }
-
@VisibleForTesting
public IServiceManager getServiceManager() {
return iServiceManager;
diff --git
a/service/src/main/java/org/apache/celeborn/server/common/service/config/DbConfigStoreImpl.java
b/service/src/main/java/org/apache/celeborn/server/common/service/config/DbConfigStoreImpl.java
new file mode 100644
index 000000000..76984d4da
--- /dev/null
+++
b/service/src/main/java/org/apache/celeborn/server/common/service/config/DbConfigStoreImpl.java
@@ -0,0 +1,31 @@
+/*
+ * 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.celeborn.server.common.service.config;
+
+public class DbConfigStoreImpl implements ConfigStore {
+
+ @Override
+ public String getName() {
+ return "DB";
+ }
+
+ @Override
+ public String getService() {
+ return DbConfigServiceImpl.class.getName();
+ }
+}
diff --git
a/service/src/main/java/org/apache/celeborn/server/common/service/config/DynamicConfigServiceFactory.java
b/service/src/main/java/org/apache/celeborn/server/common/service/config/DynamicConfigServiceFactory.java
index ede3860b9..592a7e17a 100644
---
a/service/src/main/java/org/apache/celeborn/server/common/service/config/DynamicConfigServiceFactory.java
+++
b/service/src/main/java/org/apache/celeborn/server/common/service/config/DynamicConfigServiceFactory.java
@@ -17,9 +17,7 @@
package org.apache.celeborn.server.common.service.config;
-import java.io.IOException;
-import java.util.HashMap;
-import java.util.Locale;
+import java.util.ServiceLoader;
import org.apache.celeborn.common.CelebornConf;
import org.apache.celeborn.common.util.Utils;
@@ -27,16 +25,7 @@ import org.apache.celeborn.common.util.Utils;
public class DynamicConfigServiceFactory {
private static volatile ConfigService _INSTANCE;
- // short names for dynamic config store backends
- private static final HashMap<String, String>
dynamicConfigStoreBackendShortNames =
- new HashMap<String, String>() {
- {
- put("FS", FsConfigServiceImpl.class.getName());
- put("DB", DbConfigServiceImpl.class.getName());
- }
- };
-
- public static ConfigService getConfigService(CelebornConf celebornConf)
throws IOException {
+ public static ConfigService getConfigService(CelebornConf celebornConf) {
if (celebornConf.dynamicConfigStoreBackend().isEmpty()) {
return null;
}
@@ -44,13 +33,14 @@ public class DynamicConfigServiceFactory {
if (_INSTANCE == null) {
synchronized (DynamicConfigServiceFactory.class) {
if (_INSTANCE == null) {
- String configStoreBackendName =
celebornConf.dynamicConfigStoreBackend().get();
- String configStoreBackendClass =
- dynamicConfigStoreBackendShortNames.getOrDefault(
- configStoreBackendName.toUpperCase(Locale.ROOT),
configStoreBackendName);
-
- _INSTANCE =
-
Utils.instantiateDynamicConfigStoreBackend(configStoreBackendClass,
celebornConf);
+ String configStoreBackend =
celebornConf.dynamicConfigStoreBackend().get();
+ for (ConfigStore configStore :
ServiceLoader.load(ConfigStore.class)) {
+ if (configStore.getName().equalsIgnoreCase(configStoreBackend)) {
+ configStoreBackend = configStore.getService();
+ break;
+ }
+ }
+ _INSTANCE =
Utils.instantiateDynamicConfigStoreBackend(configStoreBackend, celebornConf);
}
}
}
diff --git
a/service/src/main/java/org/apache/celeborn/server/common/service/config/FsConfigServiceImpl.java
b/service/src/main/java/org/apache/celeborn/server/common/service/config/FsConfigServiceImpl.java
index 965417443..3085d34bf 100644
---
a/service/src/main/java/org/apache/celeborn/server/common/service/config/FsConfigServiceImpl.java
+++
b/service/src/main/java/org/apache/celeborn/server/common/service/config/FsConfigServiceImpl.java
@@ -104,9 +104,4 @@ public class FsConfigServiceImpl extends
BaseConfigServiceImpl implements Config
}
return configFile;
}
-
- @Override
- public String getName() {
- return "FS";
- }
}
diff --git
a/service/src/main/java/org/apache/celeborn/server/common/service/config/FsConfigStoreImpl.java
b/service/src/main/java/org/apache/celeborn/server/common/service/config/FsConfigStoreImpl.java
new file mode 100644
index 000000000..a4ffa4b62
--- /dev/null
+++
b/service/src/main/java/org/apache/celeborn/server/common/service/config/FsConfigStoreImpl.java
@@ -0,0 +1,31 @@
+/*
+ * 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.celeborn.server.common.service.config;
+
+public class FsConfigStoreImpl implements ConfigStore {
+
+ @Override
+ public String getName() {
+ return "FS";
+ }
+
+ @Override
+ public String getService() {
+ return FsConfigServiceImpl.class.getName();
+ }
+}
diff --git
a/service/src/main/resources/META-INF/services/org.apache.celeborn.server.common.service.config.ConfigStore
b/service/src/main/resources/META-INF/services/org.apache.celeborn.server.common.service.config.ConfigStore
new file mode 100644
index 000000000..2f6072c67
--- /dev/null
+++
b/service/src/main/resources/META-INF/services/org.apache.celeborn.server.common.service.config.ConfigStore
@@ -0,0 +1,19 @@
+#
+# 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.
+#
+
+org.apache.celeborn.server.common.service.config.FsConfigStoreImpl
+org.apache.celeborn.server.common.service.config.DbConfigStoreImpl
