Skip to content

Conversation

@erenavsarogullari
Copy link
Member

@erenavsarogullari erenavsarogullari commented Feb 19, 2017

What changes were proposed in this pull request?

Fair Scheduler can be built via one of the following options:

  • By setting a spark.scheduler.allocation.file property,
  • By setting fairscheduler.xml into classpath.

These options are checked in order and fair-scheduler is built via first found option. If invalid path is found, FileNotFoundException will be expected.

This PR aims unit test coverage of these use cases and a minor documentation change has been added for second option(fairscheduler.xml into classpath) to inform the users.

Also, this PR was related with #16813 and has been created separately to keep patch content as isolated and to help the reviewers.

How was this patch tested?

Added new Unit Tests.

@erenavsarogullari
Copy link
Member Author

@kayousterhout
Copy link
Contributor

Jenkins this is ok to test

@kayousterhout
Copy link
Contributor

@erenavsarogullari there are currently a bunch of higher priority outstanding scheduler PRs, so I'm guessing it will take a while for anyone to get a chance to review this, just so you know.

@SparkQA
Copy link

SparkQA commented Feb 24, 2017

Test build #73367 has finished for PR 16992 at commit 0b38e69.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@jiangxb1987
Copy link
Contributor

Could you please bring this up-to-date? @erenavsarogullari

@SparkQA
Copy link

SparkQA commented Jul 22, 2017

Test build #79854 has finished for PR 16992 at commit 2e5afe4.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@erenavsarogullari
Copy link
Member Author

Hi @kayousterhout @markhamstra @squito,
This PR is ready to review.
Thanks in advance for all feedbacks.

Copy link
Contributor

@squito squito left a comment

Choose a reason for hiding this comment

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

@erenavsarogullari sorry this has taken forever to review! lgtm, just a super tiny nit on wording in the docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

super nit:

... and either putting a file named fairscheduler.xml on the classpath, or setting spark.scheduler.allocation.file ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed.

@squito
Copy link
Contributor

squito commented Aug 18, 2017

been a while so lets run tests again just to check:
Jenkins, test this please

@SparkQA
Copy link

SparkQA commented Aug 21, 2017

Test build #3893 has finished for PR 16992 at commit 2e5afe4.

  • This patch passes all tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@erenavsarogullari
Copy link
Member Author

Hi @squito,

Thanks for the review this patch. It is ready to re-review / merge.

@SparkQA
Copy link

SparkQA commented Aug 27, 2017

Test build #81168 has finished for PR 16992 at commit 77cfb03.

  • This patch fails SparkR unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Aug 27, 2017

Test build #81169 has finished for PR 16992 at commit 3d6c80b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@squito
Copy link
Contributor

squito commented Aug 28, 2017

merged to master

thanks @erenavsarogullari , sorry again for the delays

@asfgit asfgit closed this in 73e64f7 Aug 28, 2017
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.

5 participants