Skip to content

SPARK-1469: Scheduler mode should accept lower-case definitions and have... - #388

Closed
techaddict wants to merge 1 commit into
apache:masterfrom
techaddict:1469
Closed

techaddict wants to merge 1 commit into
apache:masterfrom
techaddict:1469

Conversation

@techaddict

Copy link
Copy Markdown
Contributor

... nicer error messages

There are two improvements to Scheduler Mode:

  1. Made the built in ones case insensitive (fair/FAIR, fifo/FIFO).
  2. If an invalid mode is given we should print a better error message.

@AmplabJenkins

Copy link
Copy Markdown

Can one of the admins verify this patch?

@techaddict

Copy link
Copy Markdown
Contributor Author

@pwendell can you review this ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good start, but if you look at the JIRA, this exception won't actually echo back to the user the name they provided, which is bad form. I think you should capture the argument the user provided first then echo it back to them:

private val schedulingModeConf = conf.get("spark.scheduler.mode", "FIFO")
val schedulingMode: SchedulingMode = try {
    SchedulingMode.withName(schedulingModeConf).toUpperCase)
  } catch {
   case e: java.util.NoSuchElementException =>
     throw new SparkException(s"unrecognized spark.scheduler.mode: $schedulingModeConf")
  }

Don't even bother re-sending the NoSuchElementException... it doesn't convey anything useful to the user.

…ave nicer error messages

There are  two improvements to Scheduler Mode:
1. Made the built in ones case insensitive (fair/FAIR, fifo/FIFO).
2. If an invalid mode is given we should print a better error message.
@pwendell

Copy link
Copy Markdown
Contributor

Jenkins, test this please.

@AmplabJenkins

Copy link
Copy Markdown

Merged build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Merged build started.

@AmplabJenkins

Copy link
Copy Markdown

Merged build finished. All automated tests passed.

@AmplabJenkins

Copy link
Copy Markdown

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14177/

@pwendell

Copy link
Copy Markdown
Contributor

Cool - thanks for this!

@pwendell

Copy link
Copy Markdown
Contributor

I've merged this.

@asfgit asfgit closed this in e269c24 Apr 16, 2014
asfgit pushed a commit that referenced this pull request Apr 16, 2014
…ave...

... nicer error messages

There are  two improvements to Scheduler Mode:
1. Made the built in ones case insensitive (fair/FAIR, fifo/FIFO).
2. If an invalid mode is given we should print a better error message.

Author: Sandeep <[email protected]>

Closes #388 from techaddict/1469 and squashes the following commits:

a31bbd5 [Sandeep] SPARK-1469: Scheduler mode should accept lower-case definitions and have nicer error messages There are  two improvements to Scheduler Mode: 1. Made the built in ones case insensitive (fair/FAIR, fifo/FIFO). 2. If an invalid mode is given we should print a better error message.
(cherry picked from commit e269c24)

Signed-off-by: Patrick Wendell <[email protected]>
pdeyhim pushed a commit to pdeyhim/spark-1 that referenced this pull request Jun 25, 2014
…ave...

... nicer error messages

There are  two improvements to Scheduler Mode:
1. Made the built in ones case insensitive (fair/FAIR, fifo/FIFO).
2. If an invalid mode is given we should print a better error message.

Author: Sandeep <[email protected]>

Closes apache#388 from techaddict/1469 and squashes the following commits:

a31bbd5 [Sandeep] SPARK-1469: Scheduler mode should accept lower-case definitions and have nicer error messages There are  two improvements to Scheduler Mode: 1. Made the built in ones case insensitive (fair/FAIR, fifo/FIFO). 2. If an invalid mode is given we should print a better error message.
erikerlandson pushed a commit to erikerlandson/spark that referenced this pull request Jul 28, 2017
bzhaoopenstack pushed a commit to bzhaoopenstack/spark that referenced this pull request Sep 11, 2019
Change flavor to boot server in FusionCloud job
RolatZhang pushed a commit to RolatZhang/spark that referenced this pull request Mar 18, 2022
* KE-34191 replace partition Table path

* change pom version
MaxGekk added a commit to MaxGekk/spark that referenced this pull request Sep 26, 2026
### What changes were proposed in this pull request?

Task 198's quiet run, which apache#388 merged without: `VarkaSharedPrefixBenchmark` regenerated on the quiet laptop at both widths, a band from five repeats, and `PLAN_TASK_198.md` 6 scoring the three predictions.

- **The computed-once prefix is admitted, and row 200 with it.** Sixty groups cost 2.7 times the default twelve on the wide run (2.5 narrow), and the default's repeated prefixes are at most about 40% of its time (33% narrow), above the 20% the admission asked for. It is a ceiling, since each extra group also reloads the column and runs its own loop.
- **The cheap-tail cliff is decided per JVM run, not by grouping.** The same kernel ran fast in some runs and about sixty times slower in others; the one-group kernel was fast in all five band runs and slow in the regeneration's single run, and the pattern reverses at 128 bits. Row 209 now starts from the JVM's own evidence rather than timings, and the committed cheap-tail rows are marked as a slow-mode run.

The branch also carries apache#428's fix, so that its quote check passes before apache#428 merges.

### Why are the changes needed?

The admission check is the decision this task exists to make, and apache#388 merged before the quiet window.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

The canary passed before the run; the quote check and the Varka pre-commit checks pass.

### Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 5.5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants