Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-26082][MESOS] Fix mesos fetch cache config name #23734

Closed
wants to merge 1 commit into from
Closed

[SPARK-26082][MESOS] Fix mesos fetch cache config name #23734

wants to merge 1 commit into from

Conversation

mwlon
Copy link

@mwlon mwlon commented Feb 4, 2019 •
edited by dongjoon-hyun
Loading

What changes were proposed in this pull request?

  • change MesosClusterScheduler to use correct argument name for Mesos fetch cache (spark.mesos.fetchCache.enable -> spark.mesos.fetcherCache.enable)

How was this patch tested?

Not sure this requires a test, since it's just a string change.

@dongjoon-hyun
Copy link
Member

ok to test

@dongjoon-hyun dongjoon-hyun changed the title change mesos fetch cache to correct name given in documentation [SPARK-26082][MESOS] Fix mesos fetch cache config name Feb 7, 2019
dongjoon-hyun
dongjoon-hyun reviewed Feb 7, 2019
View reviewed changes
...rs/mesos/src/main/scala/org/apache/spark/scheduler/cluster/mesos/MesosClusterScheduler.scala
@@ -130,7 +130,7 @@ private[spark] class MesosClusterScheduler(
private val queuedCapacity = conf.getInt("spark.mesos.maxDrivers", 200)
private val retainedDrivers = conf.getInt("spark.mesos.retainedDrivers", 200)
private val maxRetryWaitTime = conf.getInt("spark.mesos.cluster.retry.wait.max", 60) // 1 minute
private val useFetchCache = conf.getBoolean("spark.mesos.fetchCache.enable", false)
private val useFetchCache = conf.getBoolean("spark.mesos.fetcherCache.enable", false)
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for catching this.

@dongjoon-hyun
Copy link
Member

Thank you for your first contribution, @mwlon .

@SparkQA
Copy link

SparkQA commented Feb 7, 2019

Test build #102073 has finished for PR 23734 at commit 7104e3a.

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

dongjoon-hyun
dongjoon-hyun approved these changes Feb 7, 2019
View reviewed changes
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment •
edited
Loading

Choose a reason for hiding this comment

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

+1, LGTM. Merged to master/2.4/2.3.

dongjoon-hyun pushed a commit that referenced this pull request Feb 7, 2019
@dongjoon-hyun
[SPARK-26082][MESOS] Fix mesos fetch cache config name
c41a5e1 
## What changes were proposed in this pull request?

* change MesosClusterScheduler to use correct argument name for Mesos fetch cache (spark.mesos.fetchCache.enable -> spark.mesos.fetcherCache.enable)

## How was this patch tested?

Not sure this requires a test, since it's just a string change.

Closes #23734 from mwlon/SPARK-26082.

Authored-by: mwlon <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit c0811e8)
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun pushed a commit that referenced this pull request Feb 7, 2019
@dongjoon-hyun
[SPARK-26082][MESOS] Fix mesos fetch cache config name
bb6dbd2 
## What changes were proposed in this pull request?

* change MesosClusterScheduler to use correct argument name for Mesos fetch cache (spark.mesos.fetchCache.enable -> spark.mesos.fetcherCache.enable)

## How was this patch tested?

Not sure this requires a test, since it's just a string change.

Closes #23734 from mwlon/SPARK-26082.

Authored-by: mwlon <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit c0811e8)
Signed-off-by: Dongjoon Hyun <[email protected]>
This was referenced Feb 9, 2019
Closed
Closed
dongjoon-hyun pushed a commit that referenced this pull request Feb 10, 2019
@HeartSaVioR @dongjoon-hyun
[SPARK-26082][MESOS][FOLLOWUP][BRANCH-2.4] Add UT on fetcher cache op…
af3c711 
…tion on MesosClusterScheduler

## What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While #23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

## How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when #23734 is not applied and succeeds when #23734 is applied.

Closes #23753 from HeartSaVioR/SPARK-26082-branch-2.4.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
dongjoon-hyun pushed a commit that referenced this pull request Feb 10, 2019
@HeartSaVioR @dongjoon-hyun
[SPARK-26082][MESOS][FOLLOWUP][BRANCH-2.3] Add UT on fetcher cache op…
02e9890 
…tion on MesosClusterScheduler

## What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While #23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

## How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when #23734 is not applied and succeeds when #23734 is applied.

Closes #23754 from HeartSaVioR/SPARK-26082-branch-2.3.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
@jackylee-ch
[SPARK-26082][MESOS] Fix mesos fetch cache config name
a95c783 
## What changes were proposed in this pull request?

* change MesosClusterScheduler to use correct argument name for Mesos fetch cache (spark.mesos.fetchCache.enable -> spark.mesos.fetcherCache.enable)

## How was this patch tested?

Not sure this requires a test, since it's just a string change.

Closes apache#23734 from mwlon/SPARK-26082.

Authored-by: mwlon <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 23, 2019
@kai-chi
[SPARK-26082][MESOS] Fix mesos fetch cache config name
157f5e1 
## What changes were proposed in this pull request?

* change MesosClusterScheduler to use correct argument name for Mesos fetch cache (spark.mesos.fetchCache.enable -> spark.mesos.fetcherCache.enable)

## How was this patch tested?

Not sure this requires a test, since it's just a string change.

Closes apache#23734 from mwlon/SPARK-26082.

Authored-by: mwlon <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit c0811e8)
Signed-off-by: Dongjoon Hyun <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 23, 2019
@HeartSaVioR @kai-chi
[SPARK-26082][MESOS][FOLLOWUP][BRANCH-2.4] Add UT on fetcher cache op…
5719090 
…tion on MesosClusterScheduler

## What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While apache#23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

## How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when apache#23734 is not applied and succeeds when apache#23734 is applied.

Closes apache#23753 from HeartSaVioR/SPARK-26082-branch-2.4.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 25, 2019
@kai-chi
[SPARK-26082][MESOS] Fix mesos fetch cache config name
bf3b616 
## What changes were proposed in this pull request?

* change MesosClusterScheduler to use correct argument name for Mesos fetch cache (spark.mesos.fetchCache.enable -> spark.mesos.fetcherCache.enable)

## How was this patch tested?

Not sure this requires a test, since it's just a string change.

Closes apache#23734 from mwlon/SPARK-26082.

Authored-by: mwlon <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit c0811e8)
Signed-off-by: Dongjoon Hyun <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 25, 2019
@HeartSaVioR @kai-chi
[SPARK-26082][MESOS][FOLLOWUP][BRANCH-2.4] Add UT on fetcher cache op…
6d9c006 
…tion on MesosClusterScheduler

## What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While apache#23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

## How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when apache#23734 is not applied and succeeds when apache#23734 is applied.

Closes apache#23753 from HeartSaVioR/SPARK-26082-branch-2.4.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Aug 1, 2019
@kai-chi
[SPARK-26082][MESOS] Fix mesos fetch cache config name
2b07c1c 
## What changes were proposed in this pull request?

* change MesosClusterScheduler to use correct argument name for Mesos fetch cache (spark.mesos.fetchCache.enable -> spark.mesos.fetcherCache.enable)

## How was this patch tested?

Not sure this requires a test, since it's just a string change.

Closes apache#23734 from mwlon/SPARK-26082.

Authored-by: mwlon <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit c0811e8)
Signed-off-by: Dongjoon Hyun <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Aug 1, 2019
@HeartSaVioR @kai-chi
[SPARK-26082][MESOS][FOLLOWUP][BRANCH-2.4] Add UT on fetcher cache op…
0b5815d 
…tion on MesosClusterScheduler

## What changes were proposed in this pull request?

This patch adds UT on testing SPARK-26082 to avoid regression. While apache#23743 reduces the possibility to make a similar mistake, the needed lines of code for adding tests are not that huge, so I guess it might be worth to add them.

## How was this patch tested?

Newly added UTs. Test "supports setting fetcher cache" fails when apache#23734 is not applied and succeeds when apache#23734 is applied.

Closes apache#23753 from HeartSaVioR/SPARK-26082-branch-2.4.

Authored-by: Jungtaek Lim (HeartSaVioR) <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dongjoon-hyun dongjoon-hyun dongjoon-hyun approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mwlon @dongjoon-hyun @SparkQA