[GitHub] [spark] mridulm commented on a diff in pull request #39703: [SPARK-42157][CORE] `spark.scheduler.mode=FAIR` should provide FAIR scheduler
mridulm commented on code in PR #39703:
URL: https://github.com/apache/spark/pull/39703#discussion_r1084861628
##
core/src/main/scala/org/apache/spark/scheduler/SchedulableBuilder.scala:
##
@@ -86,9 +86,11 @@ private[spark] class FairSchedulableBuilder(val rootPool:
Pool, sc: SparkContext
logInfo(s"Creating Fair Scheduler pools from default file:
$DEFAULT_SCHEDULER_FILE")
Some((is, DEFAULT_SCHEDULER_FILE))
} else {
- logWarning("Fair Scheduler configuration file not found so jobs will
be scheduled in " +
-s"FIFO order. To use fair scheduling, configure pools in
$DEFAULT_SCHEDULER_FILE or " +
-s"set ${SCHEDULER_ALLOCATION_FILE.key} to a file that contains the
configuration.")
+ val schedulingMode =
SchedulingMode.withName(sc.conf.get(SCHEDULER_MODE))
+ rootPool.addSchedulable(new Pool(
+DEFAULT_POOL_NAME, schedulingMode, DEFAULT_MINIMUM_SHARE,
DEFAULT_WEIGHT))
+ logInfo("Created pool: %s, schedulingMode: %s, minShare: %d, weight:
%d".format(
Review Comment:
```suggestion
logInfo("Fair scheduler configuration not found, created default
pool: %s, schedulingMode: %s, minShare: %d, weight: %d".format(
```
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] mridulm commented on a diff in pull request #39703: [SPARK-42157][CORE] `spark.scheduler.mode=FAIR` should provide FAIR scheduler
mridulm commented on code in PR #39703:
URL: https://github.com/apache/spark/pull/39703#discussion_r1083660245
##
conf/fairscheduler-default.xml.template:
##
@@ -0,0 +1,26 @@
+
+
+
+
+
+
+FAIR
+1
+0
+
+
Review Comment:
There is a `conf/fairscheduler.xml.template` - why do we need this ?
If it is for testing, move it as a resource there instead of in conf ?
##
core/src/main/scala/org/apache/spark/scheduler/SchedulableBuilder.scala:
##
@@ -86,10 +87,17 @@ private[spark] class FairSchedulableBuilder(val rootPool:
Pool, sc: SparkContext
logInfo(s"Creating Fair Scheduler pools from default file:
$DEFAULT_SCHEDULER_FILE")
Some((is, DEFAULT_SCHEDULER_FILE))
} else {
- logWarning("Fair Scheduler configuration file not found so jobs will
be scheduled in " +
-s"FIFO order. To use fair scheduling, configure pools in
$DEFAULT_SCHEDULER_FILE or " +
-s"set ${SCHEDULER_ALLOCATION_FILE.key} to a file that contains the
configuration.")
- None
+ val is =
Utils.getSparkClassLoader.getResourceAsStream(DEFAULT_SCHEDULER_TEMPLATE_FILE)
+ if (is != null) {
+logInfo("Creating Fair Scheduler pools from default template file:
" +
+ s"$DEFAULT_SCHEDULER_TEMPLATE_FILE.")
+Some((is, DEFAULT_SCHEDULER_TEMPLATE_FILE))
+ } else {
+logWarning("Fair Scheduler configuration file not found so jobs
will be scheduled in " +
+ s"FIFO order. To use fair scheduling, configure pools in
$DEFAULT_SCHEDULER_FILE " +
+ s"or set ${SCHEDULER_ALLOCATION_FILE.key} to a file that
contains the configuration.")
+None
+ }
Review Comment:
We should not be relying on template file - in deployments, template file
can be invalid - admin's are not expecting it to be read by spark.
Instead, why not simply rely on returning `None` here ?
Note - if this is only for testing, we can special case it that way via
`spark.testing`
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
