[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user heary-cao closed the pull request at: https://github.com/apache/spark/pull/18555 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user heary-cao commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r166171819
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -231,6 +315,9 @@ package object config {
private[spark] val BLOCK_MANAGER_PORT =
ConfigBuilder("spark.blockManager.port")
.doc("Port to use for the block manager when a more specific setting
is not provided.")
.intConf
+.checkValue(v => v == 0 || (1024 <= v && v < 65536),
--- End diff --
well, thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r166164886
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -231,6 +315,9 @@ package object config {
private[spark] val BLOCK_MANAGER_PORT =
ConfigBuilder("spark.blockManager.port")
.doc("Port to use for the block manager when a more specific setting
is not provided.")
.intConf
+.checkValue(v => v == 0 || (1024 <= v && v < 65536),
--- End diff --
Ah, do you mean the check was already there and the current change just
brings it ahead to make it fail fast? If so, that's fine.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user heary-cao commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r166164325
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -231,6 +315,9 @@ package object config {
private[spark] val BLOCK_MANAGER_PORT =
ConfigBuilder("spark.blockManager.port")
.doc("Port to use for the block manager when a more specific setting
is not provided.")
.intConf
+.checkValue(v => v == 0 || (1024 <= v && v < 65536),
--- End diff --
@jerryshao , @HyukjinKwon
Because of the same port control at startServiceOnPort, do we need to open
it here?
def startServiceOnPort[T](
startPort: Int,
startService: Int => (T, Int),
conf: SparkConf,
serviceName: String = ""): (T, Int) = {
require(startPort == 0 || (1024 <= startPort && startPort < 65536),
"startPort should be between 1024 and 65535 (inclusive), or 0 for a
random free port.")
...
}
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r165838720
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -231,6 +315,9 @@ package object config {
private[spark] val BLOCK_MANAGER_PORT =
ConfigBuilder("spark.blockManager.port")
.doc("Port to use for the block manager when a more specific setting
is not provided.")
.intConf
+.checkValue(v => v == 0 || (1024 <= v && v < 65536),
--- End diff --
+1 on ^
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r129946652
--- Diff: core/src/test/scala/org/apache/spark/SparkConfSuite.scala ---
@@ -322,6 +324,291 @@ class SparkConfSuite extends SparkFunSuite with
LocalSparkContext with ResetSyst
conf.validateSettings()
}
+ test("verify spark.blockManager.port configuration") {
+val conf = new SparkConf(false)
+ .setMaster("local").setAppName("My app")
+
+conf.validateSettings()
+assert(!conf.contains(BLOCK_MANAGER_PORT.key))
+
+Seq(
+ "0", // normal values
+ "1024", // min values
+ "65535" // max values
+).foreach { value =>
+ conf.set(BLOCK_MANAGER_PORT.key, value)
+ var sc0 = new SparkContext(conf)
+ assert(sc0.isStopped === false)
+ assert(sc0.conf.get(BLOCK_MANAGER_PORT) === value.toInt)
+ sc0.stop()
+ conf.remove(BLOCK_MANAGER_PORT)
+}
+
+// Verify abnormal values
+Seq(
+ "-1",
+ "1000",
+ "65536"
+).foreach { value =>
+ conf.set(BLOCK_MANAGER_PORT.key, value)
+ val excMsg = intercept[IllegalArgumentException] {
+new SparkContext(conf)
+ }.getMessage
+ // Caused by: java.lang.IllegalArgumentException:
+ // blockManager port should be between 1024 and 65535 (inclusive),
+ // or 0 for a random free port.
+ assert(excMsg.contains("blockManager port should be between 1024 " +
+"and 65535 (inclusive), or 0 for a random free port."))
+
+ conf.remove(BLOCK_MANAGER_PORT)
+}
+ }
+
+ test("verify spark.executor.memory configuration exception") {
+val conf = new SparkConf(false)
+ .setMaster("local").setAppName("executor memory")
+ .set(EXECUTOR_MEMORY.key, "-1")
+val excMsg = intercept[NumberFormatException] {
+ sc = new SparkContext(conf)
+}.getMessage
+// Caused by: java.lang.NumberFormatException:
+// Size must be specified as bytes (b), kibibytes (k),
+// mebibytes (m), gibibytes (g), tebibytes (t),
+// or pebibytes(p). E.g. 50b, 100k, or 250m.
+assert(excMsg.contains("Size must be specified as bytes (b), kibibytes
(k), " +
+ "mebibytes (m), gibibytes (g), tebibytes (t), or pebibytes(p). E.g.
50b, 100k, or 250m."))
+ }
+
+ test("verify spark.task.cpus configuration exception") {
+val conf = new SparkConf(false)
+ .setMaster("local").setAppName("cpus")
+ .set(CPUS_PER_TASK.key, "-1")
+val excMsg = intercept[IllegalArgumentException] {
+ sc = new SparkContext(conf)
+}.getMessage
+// Caused by: java.lang.IllegalArgumentException:
+// Number of cores to allocate for task event queue must be positive.
+assert(excMsg.contains("Number of cores to allocate for task event
queue must be positive."))
+ }
+
+ test("verify spark.task.maxFailures configuration exception") {
+val conf = new SparkConf(false)
+ .setMaster("local").setAppName("task maxFailures")
+ .set(MAX_TASK_FAILURES.key, "-1")
+val sc0 = new SparkContext(conf)
+val excMsg = intercept[IllegalArgumentException] {
+ new TaskSchedulerImpl(sc0)
+}.getMessage
+// Caused by: java.lang.IllegalArgumentException:
+// The retry times of task should be greater than or equal to 1.
+assert(excMsg.contains("The retry times of task should be greater than
or equal to 1."))
+sc0.stop()
+ }
+
+ test("verify listenerbus.eventqueue.capacity configuration exception") {
+val conf = new SparkConf(false)
+ .setMaster("local").setAppName("capacity")
+ .set(LISTENER_BUS_EVENT_QUEUE_CAPACITY.key, "-1")
+val excMsg = intercept[IllegalArgumentException] {
+ sc = new SparkContext(conf)
+}.getMessage
+// Caused by: java.lang.IllegalArgumentException:
+// The capacity of listener bus event queue must be positive.
+assert(excMsg.contains("The capacity of listener bus event queue must
be positive."))
+ }
+
+ test("verify metrics.maxListenerClassesTimed configuration exception") {
+val conf = new SparkConf(false)
+ .setMaster("local").setAppName("listenerbus")
+ .set(LISTENER_BUS_METRICS_MAX_LISTENER_CLASSES_TIMED.key, "-1")
+val excMsg = intercept[IllegalArgumentException] {
+ sc = new SparkContext(conf)
+}.getMessage
+// Caused by: java.lang.IllegalArgumentException:
+// The maxListenerClassesTimed of listener bus event queue must be
positive.
+assert(excMsg.contains("The maxListenerClassesTimed of listener bus " +
+ "event queue must be
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126660503
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +62,65 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 512, 1g, 2g,
8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Int.MaxValue / 1024,
+ "The executor memory must be greater than 0 MiB " +
+ s"and less than ${Int.MaxValue / 1024} MiB.")
.createWithDefaultString("1g")
- private[spark] val IS_PYTHON_APP =
ConfigBuilder("spark.yarn.isPython").internal()
-.booleanConf.createWithDefault(false)
+ private[spark] val IS_PYTHON_APP = ConfigBuilder("spark.yarn.isPython")
+.internal()
+.booleanConf
+.createWithDefault(false)
- private[spark] val CPUS_PER_TASK =
ConfigBuilder("spark.task.cpus").intConf.createWithDefault(1)
+ private[spark] val CPUS_PER_TASK = ConfigBuilder("spark.task.cpus")
+.doc("Number of cores to allocate for each task. ")
+.intConf
+.checkValue(_ > 0,
+ "Number of cores to allocate for task event queue must be positive.")
+.createWithDefault(1)
private[spark] val DYN_ALLOCATION_MIN_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.minExecutors").intConf.createWithDefault(0)
+ConfigBuilder("spark.dynamicAllocation.minExecutors")
+ .doc("Lower bound for the number of executors if dynamic allocation
is enabled.")
+ .intConf
+ .checkValue(v => v >= 0,
+"Lower bound for the number of executors should be greater than or
equal to 0.")
+ .createWithDefault(0)
private[spark] val DYN_ALLOCATION_INITIAL_EXECUTORS =
ConfigBuilder("spark.dynamicAllocation.initialExecutors")
+ .doc("Initial number of executors to run if dynamic allocation is
enabled. " +
+"If `--num-executors` (or `spark.executor.instances`) is set " +
+"and larger than this value, " +
+"it will be used as the initial number of executors.")
.fallbackConf(DYN_ALLOCATION_MIN_EXECUTORS)
private[spark] val DYN_ALLOCATION_MAX_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.maxExecutors").intConf.createWithDefault(Int.MaxValue)
+ConfigBuilder("spark.dynamicAllocation.maxExecutors")
+ .doc("Upper bound for the number of executors if dynamic allocation
is enabled. ")
--- End diff --
nit: remove space after `.` if no additional string.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126660036
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +62,65 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 512, 1g, 2g,
8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Int.MaxValue / 1024,
+ "The executor memory must be greater than 0 MiB " +
+ s"and less than ${Int.MaxValue / 1024} MiB.")
.createWithDefaultString("1g")
- private[spark] val IS_PYTHON_APP =
ConfigBuilder("spark.yarn.isPython").internal()
-.booleanConf.createWithDefault(false)
+ private[spark] val IS_PYTHON_APP = ConfigBuilder("spark.yarn.isPython")
+.internal()
+.booleanConf
+.createWithDefault(false)
- private[spark] val CPUS_PER_TASK =
ConfigBuilder("spark.task.cpus").intConf.createWithDefault(1)
+ private[spark] val CPUS_PER_TASK = ConfigBuilder("spark.task.cpus")
+.doc("Number of cores to allocate for each task. ")
--- End diff --
nit: remove space after `.`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/18555#discussion_r126614860 --- Diff: docs/configuration.md --- @@ -142,7 +142,7 @@ of the most common options to set are: 1g Amount of memory to use for the driver process, i.e. where SparkContext is initialized. -(e.g. 1g, 2g). +(e.g. 512, 1g, 2g). --- End diff -- I'd like to revert this change, to encourage users always put a unit --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user heary-cao commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126343554
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +62,61 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 512, 1g, 2g,
8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Long.MaxValue / (1024 * 1024),
+ "The executor memory must be greater than 0 MiB " +
+ s"and less than ${Long.MaxValue / (1024 * 1024)} MiB.")
.createWithDefaultString("1g")
- private[spark] val IS_PYTHON_APP =
ConfigBuilder("spark.yarn.isPython").internal()
-.booleanConf.createWithDefault(false)
+ private[spark] val IS_PYTHON_APP = ConfigBuilder("spark.yarn.isPython")
+.internal()
+.booleanConf
+.createWithDefault(false)
- private[spark] val CPUS_PER_TASK =
ConfigBuilder("spark.task.cpus").intConf.createWithDefault(1)
+ private[spark] val CPUS_PER_TASK = ConfigBuilder("spark.task.cpus")
+.doc("Number of cores to allocate for each task. ")
+.intConf
+.checkValue(_ > 0,
+ "Number of cores to allocate for task event queue must not be
negative")
+.createWithDefault(1)
private[spark] val DYN_ALLOCATION_MIN_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.minExecutors").intConf.createWithDefault(0)
+ConfigBuilder("spark.dynamicAllocation.minExecutors")
+ .doc("Lower bound for the number of executors if dynamic allocation
is enabled.")
+ .intConf
--- End diff --
thank you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126329361
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +63,66 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .checkValues(Set(false, true))
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 2g, 8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Int.MaxValue / (1024 * 1024),
+ "The executor memory must be greater than 0M " +
+ s"and less than ${Int.MaxValue / (1024 * 1024)}M.")
.createWithDefaultString("1g")
- private[spark] val IS_PYTHON_APP =
ConfigBuilder("spark.yarn.isPython").internal()
-.booleanConf.createWithDefault(false)
+ private[spark] val IS_PYTHON_APP = ConfigBuilder("spark.yarn.isPython")
+.internal()
+.booleanConf
+.checkValues(Set(false, true))
+.createWithDefault(false)
- private[spark] val CPUS_PER_TASK =
ConfigBuilder("spark.task.cpus").intConf.createWithDefault(1)
+ private[spark] val CPUS_PER_TASK = ConfigBuilder("spark.task.cpus")
+.doc("Number of cores to allocate for each task. ")
+.intConf
+.checkValue(v => v > 0,
+ "Number of cores to allocate for task event queue must not be
negative")
+.createWithDefault(1)
private[spark] val DYN_ALLOCATION_MIN_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.minExecutors").intConf.createWithDefault(0)
+ConfigBuilder("spark.dynamicAllocation.minExecutors")
+ .doc("Lower bound for the number of executors if dynamic allocation
is enabled.")
+ .intConf
+ .checkValue(v => v >= 0,
+"Lower bound for the number of executors should be greater than or
equal to 0")
+ .createWithDefault(0)
private[spark] val DYN_ALLOCATION_INITIAL_EXECUTORS =
ConfigBuilder("spark.dynamicAllocation.initialExecutors")
+ .doc("Initial number of executors to run if dynamic allocation is
enabled. " +
+"If `--num-executors` (or `spark.executor.instances`) is set " +
+"and larger than this value, " +
+"it will be used as the initial number of executors.")
.fallbackConf(DYN_ALLOCATION_MIN_EXECUTORS)
private[spark] val DYN_ALLOCATION_MAX_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.maxExecutors").intConf.createWithDefault(Int.MaxValue)
+ConfigBuilder("spark.dynamicAllocation.maxExecutors")
+ .doc("Upper bound for the number of executors if dynamic allocation
is enabled. ")
+ .intConf
+ .createWithDefault(Int.MaxValue)
--- End diff --
This is the same.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126329304
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +62,61 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 512, 1g, 2g,
8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Long.MaxValue / (1024 * 1024),
+ "The executor memory must be greater than 0 MiB " +
+ s"and less than ${Long.MaxValue / (1024 * 1024)} MiB.")
.createWithDefaultString("1g")
- private[spark] val IS_PYTHON_APP =
ConfigBuilder("spark.yarn.isPython").internal()
-.booleanConf.createWithDefault(false)
+ private[spark] val IS_PYTHON_APP = ConfigBuilder("spark.yarn.isPython")
+.internal()
+.booleanConf
+.createWithDefault(false)
- private[spark] val CPUS_PER_TASK =
ConfigBuilder("spark.task.cpus").intConf.createWithDefault(1)
+ private[spark] val CPUS_PER_TASK = ConfigBuilder("spark.task.cpus")
+.doc("Number of cores to allocate for each task. ")
+.intConf
+.checkValue(_ > 0,
+ "Number of cores to allocate for task event queue must not be
negative")
+.createWithDefault(1)
private[spark] val DYN_ALLOCATION_MIN_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.minExecutors").intConf.createWithDefault(0)
+ConfigBuilder("spark.dynamicAllocation.minExecutors")
+ .doc("Lower bound for the number of executors if dynamic allocation
is enabled.")
+ .intConf
--- End diff --
It's a SparkException check test case for validation
[ExecutorAllocationManager](https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala#L177-L179
).
I think you can replace `SparkException` with yours in
`ExecutorAllocationManagerSuite`.
```
// Min < 0
val conf1 = conf.clone().set("spark.dynamicAllocation.minExecutors",
"-1")
intercept[SparkException] { contexts += new SparkContext(conf1) }
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126329325
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +62,61 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 512, 1g, 2g,
8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Long.MaxValue / (1024 * 1024),
+ "The executor memory must be greater than 0 MiB " +
+ s"and less than ${Long.MaxValue / (1024 * 1024)} MiB.")
.createWithDefaultString("1g")
- private[spark] val IS_PYTHON_APP =
ConfigBuilder("spark.yarn.isPython").internal()
-.booleanConf.createWithDefault(false)
+ private[spark] val IS_PYTHON_APP = ConfigBuilder("spark.yarn.isPython")
+.internal()
+.booleanConf
+.createWithDefault(false)
- private[spark] val CPUS_PER_TASK =
ConfigBuilder("spark.task.cpus").intConf.createWithDefault(1)
+ private[spark] val CPUS_PER_TASK = ConfigBuilder("spark.task.cpus")
+.doc("Number of cores to allocate for each task. ")
+.intConf
+.checkValue(_ > 0,
+ "Number of cores to allocate for task event queue must not be
negative")
+.createWithDefault(1)
private[spark] val DYN_ALLOCATION_MIN_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.minExecutors").intConf.createWithDefault(0)
+ConfigBuilder("spark.dynamicAllocation.minExecutors")
+ .doc("Lower bound for the number of executors if dynamic allocation
is enabled.")
+ .intConf
--- End diff --
Also, it would be great if you check the error message of the exception.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user heary-cao commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126326738
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +62,61 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 512, 1g, 2g,
8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Long.MaxValue / (1024 * 1024),
+ "The executor memory must be greater than 0 MiB " +
+ s"and less than ${Long.MaxValue / (1024 * 1024)} MiB.")
.createWithDefaultString("1g")
- private[spark] val IS_PYTHON_APP =
ConfigBuilder("spark.yarn.isPython").internal()
-.booleanConf.createWithDefault(false)
+ private[spark] val IS_PYTHON_APP = ConfigBuilder("spark.yarn.isPython")
+.internal()
+.booleanConf
+.createWithDefault(false)
- private[spark] val CPUS_PER_TASK =
ConfigBuilder("spark.task.cpus").intConf.createWithDefault(1)
+ private[spark] val CPUS_PER_TASK = ConfigBuilder("spark.task.cpus")
+.doc("Number of cores to allocate for each task. ")
+.intConf
+.checkValue(_ > 0,
+ "Number of cores to allocate for task event queue must not be
negative")
+.createWithDefault(1)
private[spark] val DYN_ALLOCATION_MIN_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.minExecutors").intConf.createWithDefault(0)
+ConfigBuilder("spark.dynamicAllocation.minExecutors")
+ .doc("Lower bound for the number of executors if dynamic allocation
is enabled.")
+ .intConf
--- End diff --
It might be a bit out of place.
In org.apache.spark.ExecutorAllocationManagerSuite.scala
val conf2 = conf.clone().set("spark.dynamicAllocation.minExecutors", "-1")
Why is this value set less than zero?
Not yet understood.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315874
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -327,6 +421,8 @@ package object config {
.doc("The blocks of a shuffle request will be fetched to disk when
size of the request is " +
"above this threshold. This is to avoid a giant request takes too
much memory.")
.bytesConf(ByteUnit.BYTE)
+ .checkValue(_ > 0,
+"Size of the request is above this threshold event queue must not
be negative")
--- End diff --
ditto. `positive`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315869
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -306,19 +396,23 @@ package object config {
"record the size accurately if it's above this config. This helps
to prevent OOM by " +
"avoiding underestimating shuffle block size when fetch shuffle
blocks.")
.bytesConf(ByteUnit.BYTE)
+ .checkValue(_ > 0, "The number of BlockThreshold event queue must
not be negative")
.createWithDefault(100 * 1024 * 1024)
private[spark] val SHUFFLE_REGISTRATION_TIMEOUT =
ConfigBuilder("spark.shuffle.registration.timeout")
.doc("Timeout in milliseconds for registration to the external
shuffle service.")
.timeConf(TimeUnit.MILLISECONDS)
+ .checkValue(_ > 0,
+"Timeout in milliseconds for registration event queue must not be
negative")
--- End diff --
ditto. `positive`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315870
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -306,19 +396,23 @@ package object config {
"record the size accurately if it's above this config. This helps
to prevent OOM by " +
"avoiding underestimating shuffle block size when fetch shuffle
blocks.")
.bytesConf(ByteUnit.BYTE)
+ .checkValue(_ > 0, "The number of BlockThreshold event queue must
not be negative")
.createWithDefault(100 * 1024 * 1024)
private[spark] val SHUFFLE_REGISTRATION_TIMEOUT =
ConfigBuilder("spark.shuffle.registration.timeout")
.doc("Timeout in milliseconds for registration to the external
shuffle service.")
.timeConf(TimeUnit.MILLISECONDS)
+ .checkValue(_ > 0,
+"Timeout in milliseconds for registration event queue must not be
negative")
.createWithDefault(5000)
private[spark] val SHUFFLE_REGISTRATION_MAX_ATTEMPTS =
ConfigBuilder("spark.shuffle.registration.maxAttempts")
.doc("When we fail to register to the external shuffle service, we
will " +
"retry for maxAttempts times.")
.intConf
+ .checkValue(_ > 0, "Retry for maxAttempts times event queue must not
be negative")
--- End diff --
ditto. `positive`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315864
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -306,19 +396,23 @@ package object config {
"record the size accurately if it's above this config. This helps
to prevent OOM by " +
"avoiding underestimating shuffle block size when fetch shuffle
blocks.")
.bytesConf(ByteUnit.BYTE)
+ .checkValue(_ > 0, "The number of BlockThreshold event queue must
not be negative")
--- End diff --
ditto. `positive`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315856
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -259,6 +344,7 @@ package object config {
" over estimate, then the partitions with small files will be faster
than partitions with" +
" bigger files.")
.longConf
+.checkValue(_ > 0, "The number of CostInBytes event queue must not be
negative")
--- End diff --
Since `_ > 0`, we should avoid 0, too.
`must not be negative` => `must be positive`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315844
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -183,32 +254,42 @@ package object config {
.createOptional
private[spark] val PYSPARK_DRIVER_PYTHON =
ConfigBuilder("spark.pyspark.driver.python")
+.doc("Python binary executable to use for PySpark in driver.")
.stringConf
.createOptional
private[spark] val PYSPARK_PYTHON = ConfigBuilder("spark.pyspark.python")
+.doc("Python binary executable to use for PySpark in both driver and
executors.")
.stringConf
.createOptional
// To limit memory usage, we only track information for a fixed number
of tasks
private[spark] val UI_RETAINED_TASKS =
ConfigBuilder("spark.ui.retainedTasks")
+.doc("How many tasks the Spark UI and status APIs remember before
garbage collecting.")
.intConf
+.checkValue(_ > 0, "Number of Tasks event queue must not be negative")
--- End diff --
Since `_ > 0`, we should avoid 0, too.
`must not be negative` => `must be positive`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315840
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -175,6 +244,8 @@ package object config {
ConfigBuilder("spark.scheduler.listenerbus.metrics.maxListenerClassesTimed")
.internal()
.intConf
+ .checkValue(_ > 0,
+"The maxListenerClassesTimed of listener bus event queue must not
be negative")
--- End diff --
Since `_ > 0`, we should avoid 0, too.
`must not be negative` => `must be positive`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315850
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -251,6 +335,7 @@ package object config {
private[spark] val FILES_MAX_PARTITION_BYTES =
ConfigBuilder("spark.files.maxPartitionBytes")
.doc("The maximum number of bytes to pack into a single partition when
reading files.")
.longConf
+.checkValue(_ > 0, "The maximum number of bytes event queue must not
be negative")
--- End diff --
Since `_ > 0`, we should avoid 0, too.
`must not be negative` => `must be positive`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315834
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +62,61 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 512, 1g, 2g,
8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Long.MaxValue / (1024 * 1024),
+ "The executor memory must be greater than 0 MiB " +
+ s"and less than ${Long.MaxValue / (1024 * 1024)} MiB.")
.createWithDefaultString("1g")
- private[spark] val IS_PYTHON_APP =
ConfigBuilder("spark.yarn.isPython").internal()
-.booleanConf.createWithDefault(false)
+ private[spark] val IS_PYTHON_APP = ConfigBuilder("spark.yarn.isPython")
+.internal()
+.booleanConf
+.createWithDefault(false)
- private[spark] val CPUS_PER_TASK =
ConfigBuilder("spark.task.cpus").intConf.createWithDefault(1)
+ private[spark] val CPUS_PER_TASK = ConfigBuilder("spark.task.cpus")
+.doc("Number of cores to allocate for each task. ")
+.intConf
+.checkValue(_ > 0,
+ "Number of cores to allocate for task event queue must not be
negative")
--- End diff --
Since `_ > 0`, we should avoid 0, too.
`must not be negative` => `must be positive`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315664
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +62,61 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 512, 1g, 2g,
8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Long.MaxValue / (1024 * 1024),
+ "The executor memory must be greater than 0 MiB " +
+ s"and less than ${Long.MaxValue / (1024 * 1024)} MiB.")
.createWithDefaultString("1g")
- private[spark] val IS_PYTHON_APP =
ConfigBuilder("spark.yarn.isPython").internal()
-.booleanConf.createWithDefault(false)
+ private[spark] val IS_PYTHON_APP = ConfigBuilder("spark.yarn.isPython")
+.internal()
+.booleanConf
+.createWithDefault(false)
- private[spark] val CPUS_PER_TASK =
ConfigBuilder("spark.task.cpus").intConf.createWithDefault(1)
+ private[spark] val CPUS_PER_TASK = ConfigBuilder("spark.task.cpus")
+.doc("Number of cores to allocate for each task. ")
+.intConf
+.checkValue(_ > 0,
+ "Number of cores to allocate for task event queue must not be
negative")
+.createWithDefault(1)
private[spark] val DYN_ALLOCATION_MIN_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.minExecutors").intConf.createWithDefault(0)
+ConfigBuilder("spark.dynamicAllocation.minExecutors")
+ .doc("Lower bound for the number of executors if dynamic allocation
is enabled.")
+ .intConf
--- End diff --
check value with `_ >= 0`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315636
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +62,61 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 512, 1g, 2g,
8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Long.MaxValue / (1024 * 1024),
+ "The executor memory must be greater than 0 MiB " +
+ s"and less than ${Long.MaxValue / (1024 * 1024)} MiB.")
--- End diff --
ditto.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126315617
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -35,10 +35,21 @@ package object config {
ConfigBuilder(SparkLauncher.DRIVER_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val DRIVER_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.driver.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.driver.userClassPathFirst")
+ .doc("Whether to give user-added jars precedence over Spark's own
jars " +
+"when loading classes in the driver. This feature can be used to
mitigate " +
+"conflicts between Spark's dependencies and user dependencies. " +
+"It is currently an experimental feature. This is used in cluster
mode only. ")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val DRIVER_MEMORY = ConfigBuilder("spark.driver.memory")
+.doc("Amount of memory to use for the driver process, i.e. " +
+ "where SparkContext is initialized. (e.g. 512, 1g, 2g). ")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Long.MaxValue / (1024 * 1024),
+ "The driver memory must be greater than 0 MiB " +
+ s"and less than ${Long.MaxValue / (1024 * 1024)} MiB.")
--- End diff --
nit, the maximum value for `spark.driver.memory` in a single machine looks
too unrealistic, 8191 PB?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user heary-cao commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126308305
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +63,66 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .checkValues(Set(false, true))
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 2g, 8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Int.MaxValue / (1024 * 1024),
+ "The executor memory must be greater than 0M " +
+ s"and less than ${Int.MaxValue / (1024 * 1024)}M.")
.createWithDefaultString("1g")
- private[spark] val IS_PYTHON_APP =
ConfigBuilder("spark.yarn.isPython").internal()
-.booleanConf.createWithDefault(false)
+ private[spark] val IS_PYTHON_APP = ConfigBuilder("spark.yarn.isPython")
+.internal()
+.booleanConf
+.checkValues(Set(false, true))
+.createWithDefault(false)
- private[spark] val CPUS_PER_TASK =
ConfigBuilder("spark.task.cpus").intConf.createWithDefault(1)
+ private[spark] val CPUS_PER_TASK = ConfigBuilder("spark.task.cpus")
+.doc("Number of cores to allocate for each task. ")
+.intConf
+.checkValue(v => v > 0,
+ "Number of cores to allocate for task event queue must not be
negative")
+.createWithDefault(1)
private[spark] val DYN_ALLOCATION_MIN_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.minExecutors").intConf.createWithDefault(0)
+ConfigBuilder("spark.dynamicAllocation.minExecutors")
+ .doc("Lower bound for the number of executors if dynamic allocation
is enabled.")
+ .intConf
+ .checkValue(v => v >= 0,
+"Lower bound for the number of executors should be greater than or
equal to 0")
+ .createWithDefault(0)
private[spark] val DYN_ALLOCATION_INITIAL_EXECUTORS =
ConfigBuilder("spark.dynamicAllocation.initialExecutors")
+ .doc("Initial number of executors to run if dynamic allocation is
enabled. " +
+"If `--num-executors` (or `spark.executor.instances`) is set " +
+"and larger than this value, " +
+"it will be used as the initial number of executors.")
.fallbackConf(DYN_ALLOCATION_MIN_EXECUTORS)
private[spark] val DYN_ALLOCATION_MAX_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.maxExecutors").intConf.createWithDefault(Int.MaxValue)
+ConfigBuilder("spark.dynamicAllocation.maxExecutors")
+ .doc("Upper bound for the number of executors if dynamic allocation
is enabled. ")
+ .intConf
+ .createWithDefault(Int.MaxValue)
--- End diff --
It might be a bit out of place.
In org.apache.spark.ExecutorAllocationManagerSuite.scala
`val conf2 = conf.clone().set("spark.dynamicAllocation.maxExecutors", "-1")`
Why is this value set less than zero?
Not yet understood.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user heary-cao commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126308228
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -35,10 +35,21 @@ package object config {
ConfigBuilder(SparkLauncher.DRIVER_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val DRIVER_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.driver.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.driver.userClassPathFirst")
+ .doc("Whether to give user-added jars precedence over Spark's own
jars " +
+"when loading classes in the driver. This feature can be used to
mitigate " +
+"conflicts between Spark's dependencies and user dependencies. " +
+"It is currently an experimental feature. This is used in cluster
mode only. ")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val DRIVER_MEMORY = ConfigBuilder("spark.driver.memory")
+.doc("Amount of memory to use for the driver process, i.e. " +
+ "where SparkContext is initialized. (e.g. 512, 1g, 2g). ")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Int.MaxValue / (1024 * 1024),
--- End diff --
sorry .
Int to Long.
I think MiB for 1024 * 1024.
thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #18555: [SPARK-21353][CORE]add checkValue in spark.intern...
Github user heary-cao commented on a diff in the pull request:
https://github.com/apache/spark/pull/18555#discussion_r126308199
--- Diff:
core/src/main/scala/org/apache/spark/internal/config/package.scala ---
@@ -51,29 +62,63 @@ package object config {
ConfigBuilder(SparkLauncher.EXECUTOR_EXTRA_LIBRARY_PATH).stringConf.createOptional
private[spark] val EXECUTOR_USER_CLASS_PATH_FIRST =
-
ConfigBuilder("spark.executor.userClassPathFirst").booleanConf.createWithDefault(false)
+ConfigBuilder("spark.executor.userClassPathFirst")
+ .doc("Same functionality as spark.driver.userClassPathFirst, " +
+"but applied to executor instances.")
+ .booleanConf
+ .createWithDefault(false)
private[spark] val EXECUTOR_MEMORY =
ConfigBuilder("spark.executor.memory")
+.doc("Amount of memory to use per executor process (e.g. 512, 1g, 2g,
8g).")
.bytesConf(ByteUnit.MiB)
+.checkValue(v => v > 0 && v <= Int.MaxValue / (1024 * 1024),
+ "The executor memory must be greater than 0M " +
+ s"and less than ${Int.MaxValue / (1024 * 1024)}M.")
.createWithDefaultString("1g")
- private[spark] val IS_PYTHON_APP =
ConfigBuilder("spark.yarn.isPython").internal()
-.booleanConf.createWithDefault(false)
+ private[spark] val IS_PYTHON_APP = ConfigBuilder("spark.yarn.isPython")
+.internal()
+.booleanConf
+.createWithDefault(false)
- private[spark] val CPUS_PER_TASK =
ConfigBuilder("spark.task.cpus").intConf.createWithDefault(1)
+ private[spark] val CPUS_PER_TASK = ConfigBuilder("spark.task.cpus")
+.doc("Number of cores to allocate for each task. ")
+.intConf
+.checkValue(v => v > 0,
+ "Number of cores to allocate for task event queue must not be
negative")
+.createWithDefault(1)
private[spark] val DYN_ALLOCATION_MIN_EXECUTORS =
-
ConfigBuilder("spark.dynamicAllocation.minExecutors").intConf.createWithDefault(0)
+ConfigBuilder("spark.dynamicAllocation.minExecutors")
+ .doc("Lower bound for the number of executors if dynamic allocation
is enabled.")
+ .intConf
+ .checkValue(v => v >= 0,
--- End diff --
thank you for answer it.
I have remove CheckValue.
thank you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
