You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@celeborn.apache.org by zh...@apache.org on 2023/06/02 06:58:28 UTC
[incubator-celeborn] 09/20: [CELEBORN-601] Consolidate configsWithAlternatives with `ConfigBuilder.withAlternative` (#1506)
This is an automated email from the ASF dual-hosted git repository.
zhongqiangchen pushed a commit to branch branch-0.3
in repository https://gitbox.apache.org/repos/asf/incubator-celeborn.git
commit a5a3f4e7ecaa26ad66725e87ee58e10a221feac7
Author: Angerszhuuuu <an...@gmail.com>
AuthorDate: Sun May 28 09:13:05 2023 +0800
[CELEBORN-601] Consolidate configsWithAlternatives with `ConfigBuilder.withAlternative` (#1506)
---
.../org/apache/celeborn/common/CelebornConf.scala | 28 ++++++++++++++++++++--
.../common/internal/config/ConfigBuilder.scala | 10 ++++----
.../common/internal/config/ConfigEntry.scala | 15 ++++++------
.../apache/celeborn/common/CelebornConfSuite.scala | 7 ++++++
4 files changed, 47 insertions(+), 13 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 f9a6bb622..931297e47 100644
--- a/common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
+++ b/common/src/main/scala/org/apache/celeborn/common/CelebornConf.scala
@@ -22,6 +22,7 @@ import java.util.{Collection => JCollection, Collections, HashMap => JHashMap, L
import java.util.concurrent.TimeUnit
import scala.collection.JavaConverters._
+import scala.collection.mutable
import scala.concurrent.duration._
import scala.util.Try
@@ -903,6 +904,7 @@ class CelebornConf(loadDefaults: Boolean) extends Cloneable with Logging with Se
def testPushMasterDataTimeout: Boolean = get(TEST_PUSH_MASTER_DATA_TIMEOUT)
def testPushSlaveDataTimeout: Boolean = get(TEST_PUSH_SLAVE_DATA_TIMEOUT)
def testRetryRevive: Boolean = get(TEST_RETRY_REVIVE)
+ def testAlternative: String = get(TEST_ALTERNATIVE.key, "celeborn")
}
object CelebornConf extends Logging {
@@ -979,7 +981,7 @@ object CelebornConf extends Logging {
* The alternates are used in the order defined in this map. If deprecated configs are
* present in the user's configuration, a warning is logged.
*/
- private val configsWithAlternatives = Map[String, Seq[AlternateConfig]](
+ private val configsWithAlternatives = mutable.Map[String, Seq[AlternateConfig]](
"none" -> Seq(
AlternateConfig("none", "1.0")))
@@ -995,6 +997,15 @@ object CelebornConf extends Logging {
}.toMap
}
+ private def addDeprecatedConfig(entry: ConfigEntry[_], alt: (String, String => String)): Unit = {
+ configsWithAlternatives.put(
+ entry.key,
+ configsWithAlternatives.getOrElse(entry.key, Seq.empty) :+ AlternateConfig(
+ alt._1,
+ entry.version,
+ alt._2))
+ }
+
/**
* Looks for available deprecated keys for the given config option, and return the first
* value available.
@@ -1050,6 +1061,8 @@ object CelebornConf extends Logging {
val updatedMap = new JHashMap[String, ConfigEntry[_]](confEntries)
updatedMap.put(entry.key, entry)
confEntries = updatedMap
+
+ entry.alternatives.foreach(addDeprecatedConfig(entry, _))
}
private[celeborn] def unregister(entry: ConfigEntry[_]): Unit =
@@ -1057,6 +1070,8 @@ object CelebornConf extends Logging {
val updatedMap = new JHashMap[String, ConfigEntry[_]](confEntries)
updatedMap.remove(entry.key)
confEntries = updatedMap
+
+ configsWithAlternatives.remove(entry.key)
}
private[celeborn] def getConfigEntry(key: String): ConfigEntry[_] = {
@@ -1075,7 +1090,7 @@ object CelebornConf extends Logging {
confEntries.containsKey(key)
}
- def buildConf(key: String): ConfigBuilder = ConfigBuilder(key).onCreate(register)
+ private def buildConf(key: String): ConfigBuilder = ConfigBuilder(key).onCreate(register)
val NETWORK_TIMEOUT: ConfigEntry[Long] =
buildConf("celeborn.network.timeout")
@@ -3308,4 +3323,13 @@ object CelebornConf extends Logging {
.doc("the heartbeat interval between worker and client")
.timeConf(TimeUnit.MILLISECONDS)
.createWithDefaultString("60s")
+
+ val TEST_ALTERNATIVE: OptionalConfigEntry[String] =
+ buildConf("celeborn.test.alternative.key")
+ .withAlternative("celeborn.test.alternative.deprecatedKey")
+ .categories("test")
+ .internal
+ .version("0.3.0")
+ .stringConf
+ .createOptional
}
diff --git a/common/src/main/scala/org/apache/celeborn/common/internal/config/ConfigBuilder.scala b/common/src/main/scala/org/apache/celeborn/common/internal/config/ConfigBuilder.scala
index f972643c4..76fd10f09 100644
--- a/common/src/main/scala/org/apache/celeborn/common/internal/config/ConfigBuilder.scala
+++ b/common/src/main/scala/org/apache/celeborn/common/internal/config/ConfigBuilder.scala
@@ -26,7 +26,9 @@ import org.apache.celeborn.common.CelebornConf
import org.apache.celeborn.common.network.util.ByteUnit
import org.apache.celeborn.common.util.{JavaUtils, Utils}
-private object ConfigHelpers {
+private[celeborn] object ConfigHelpers {
+
+ type AlternativesTransfer = (String, String => String)
def toNumber[T](s: String, converter: String => T, key: String, configType: String): T = {
try {
@@ -228,7 +230,7 @@ case class ConfigBuilder(key: String) {
private[config] var _categories = Seq.empty[String]
private[config] var _version = ""
private[config] var _onCreate: Option[ConfigEntry[_] => Unit] = None
- private[config] var _alternatives = List.empty[String]
+ private[config] var _alternatives = List.empty[AlternativesTransfer]
def internal: ConfigBuilder = {
_public = false
@@ -265,8 +267,8 @@ case class ConfigBuilder(key: String) {
this
}
- def withAlternative(key: String): ConfigBuilder = {
- _alternatives = _alternatives :+ key
+ def withAlternative(key: String, translate: String => String = null): ConfigBuilder = {
+ _alternatives = _alternatives :+ (key, translate)
this
}
diff --git a/common/src/main/scala/org/apache/celeborn/common/internal/config/ConfigEntry.scala b/common/src/main/scala/org/apache/celeborn/common/internal/config/ConfigEntry.scala
index 7305c68fb..5856da7f6 100644
--- a/common/src/main/scala/org/apache/celeborn/common/internal/config/ConfigEntry.scala
+++ b/common/src/main/scala/org/apache/celeborn/common/internal/config/ConfigEntry.scala
@@ -19,6 +19,7 @@ package org.apache.celeborn.common.internal.config
import java.util.concurrent.ConcurrentHashMap
+import org.apache.celeborn.common.internal.config.ConfigHelpers.AlternativesTransfer
import org.apache.celeborn.common.util.JavaUtils
// ====================================================================================
@@ -78,7 +79,7 @@ abstract class ConfigEntry[T](
val key: String,
val prependedKey: Option[String],
val prependSeparator: String,
- val alternatives: List[String],
+ val alternatives: List[AlternativesTransfer],
val valueConverter: String => T,
val stringConverter: T => String,
val doc: String,
@@ -96,7 +97,7 @@ abstract class ConfigEntry[T](
val values = Seq(
prependedKey.flatMap(reader.get),
alternatives.foldLeft(reader.get(key))((res, nextKey) =>
- res.orElse(reader.get(nextKey)))).flatten
+ res.orElse(reader.get(nextKey._1)))).flatten
if (values.nonEmpty) {
Some(values.mkString(prependSeparator))
} else {
@@ -118,7 +119,7 @@ private class ConfigEntryWithDefault[T](
key: String,
prependedKey: Option[String],
prependSeparator: String,
- alternatives: List[String],
+ alternatives: List[AlternativesTransfer],
_defaultValue: T,
valueConverter: String => T,
stringConverter: T => String,
@@ -151,7 +152,7 @@ private class ConfigEntryWithDefaultFunction[T](
key: String,
prependedKey: Option[String],
prependSeparator: String,
- alternatives: List[String],
+ alternatives: List[AlternativesTransfer],
_defaultFunction: () => T,
valueConverter: String => T,
stringConverter: T => String,
@@ -184,7 +185,7 @@ private class ConfigEntryWithDefaultString[T](
key: String,
prependedKey: Option[String],
prependSeparator: String,
- alternatives: List[String],
+ alternatives: List[AlternativesTransfer],
_defaultValue: String,
valueConverter: String => T,
stringConverter: T => String,
@@ -221,7 +222,7 @@ class OptionalConfigEntry[T](
key: String,
prependedKey: Option[String],
prependSeparator: String,
- alternatives: List[String],
+ alternatives: List[AlternativesTransfer],
val rawValueConverter: String => T,
val rawStringConverter: T => String,
doc: String,
@@ -254,7 +255,7 @@ class FallbackConfigEntry[T](
key: String,
prependedKey: Option[String],
prependSeparator: String,
- alternatives: List[String],
+ alternatives: List[AlternativesTransfer],
doc: String,
isPublic: Boolean,
categories: Seq[String],
diff --git a/common/src/test/scala/org/apache/celeborn/common/CelebornConfSuite.scala b/common/src/test/scala/org/apache/celeborn/common/CelebornConfSuite.scala
index 4032cd4e7..befd561dc 100644
--- a/common/src/test/scala/org/apache/celeborn/common/CelebornConfSuite.scala
+++ b/common/src/test/scala/org/apache/celeborn/common/CelebornConfSuite.scala
@@ -177,4 +177,11 @@ class CelebornConfSuite extends CelebornFunSuite {
assert(conf.networkConnectTimeout.duration.toMillis == 2000L)
assert(conf.networkIoConnectTimeoutMs("data") == 2000L)
}
+
+ test("CELEBORN-601: Consolidate configsWithAlternatives with `ConfigBuilder.withAlternative`") {
+ val conf = new CelebornConf()
+ .set(CelebornConf.TEST_ALTERNATIVE.alternatives.head._1, "rss")
+
+ assert(conf.testAlternative == "rss")
+ }
}