Skip to content

Conversation

@Ngone51
Copy link
Member

@Ngone51 Ngone51 commented Dec 18, 2018

What changes were proposed in this pull request?

Right now, we cancel pending allocate requests by its sending order. I thing we can take

locality preference into account when do this to perfom least impact on task locality preference.

How was this patch tested?

N.A.

@Ngone51
Copy link
Member Author

Ngone51 commented Dec 18, 2018

ping @sryza @vanzin @jerryshao

please take a look, thanks.

Copy link
Contributor

@vanzin vanzin left a comment

Choose a reason for hiding this comment

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

Please fix the title so the wrapped content is in the title.

} else {
logWarning("Expected to find pending requests, but found none.")
// cancel pending allocate requests by taking locality preference into account
val cancelRequests = {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be a single statement, since take handles negative values.

scala> Seq(1, 2, 3).take(-1)
res0: Seq[Int] = List()

@vanzin
Copy link
Contributor

vanzin commented Dec 18, 2018

ok to test

@SparkQA
Copy link

SparkQA commented Dec 18, 2018

Test build #100291 has finished for PR 23344 at commit 23f809c.

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

@Ngone51 Ngone51 changed the title [SPARK-26392][YARN] Cancel pending allocate requests by taking locality preference into a… [SPARK-26392][YARN] Cancel pending allocate requests by taking locality preference into account Dec 19, 2018
@Ngone51
Copy link
Member Author

Ngone51 commented Dec 19, 2018

Thank you for review @vanzin , updated.

@SparkQA
Copy link

SparkQA commented Dec 19, 2018

Test build #100296 has finished for PR 23344 at commit d29eb7a.

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

@vanzin
Copy link
Contributor

vanzin commented Dec 20, 2018

Merging to master.

@vanzin
Copy link
Contributor

vanzin commented Dec 20, 2018

(And also 2.4.).

@asfgit asfgit closed this in 3d6b44d Dec 20, 2018
asfgit pushed a commit that referenced this pull request Dec 20, 2018
…ty preference into account

## What changes were proposed in this pull request?

Right now, we cancel pending allocate requests by its sending order. I thing we can take

locality preference into account when do this to perfom least impact on task locality preference.

## How was this patch tested?

N.A.

Closes #23344 from Ngone51/dev-cancel-pending-allocate-requests-by-taking-locality-preference-into-account.

Authored-by: Ngone51 <[email protected]>
Signed-off-by: Marcelo Vanzin <[email protected]>
(cherry picked from commit 3d6b44d)
Signed-off-by: Marcelo Vanzin <[email protected]>
@Ngone51
Copy link
Member Author

Ngone51 commented Dec 21, 2018

Thanks! @vanzin

holdenk pushed a commit to holdenk/spark that referenced this pull request Jan 5, 2019
…ty preference into account

## What changes were proposed in this pull request?

Right now, we cancel pending allocate requests by its sending order. I thing we can take

locality preference into account when do this to perfom least impact on task locality preference.

## How was this patch tested?

N.A.

Closes apache#23344 from Ngone51/dev-cancel-pending-allocate-requests-by-taking-locality-preference-into-account.

Authored-by: Ngone51 <[email protected]>
Signed-off-by: Marcelo Vanzin <[email protected]>
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
…ty preference into account

## What changes were proposed in this pull request?

Right now, we cancel pending allocate requests by its sending order. I thing we can take

locality preference into account when do this to perfom least impact on task locality preference.

## How was this patch tested?

N.A.

Closes apache#23344 from Ngone51/dev-cancel-pending-allocate-requests-by-taking-locality-preference-into-account.

Authored-by: Ngone51 <[email protected]>
Signed-off-by: Marcelo Vanzin <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 23, 2019
…ty preference into account

## What changes were proposed in this pull request?

Right now, we cancel pending allocate requests by its sending order. I thing we can take

locality preference into account when do this to perfom least impact on task locality preference.

## How was this patch tested?

N.A.

Closes apache#23344 from Ngone51/dev-cancel-pending-allocate-requests-by-taking-locality-preference-into-account.

Authored-by: Ngone51 <[email protected]>
Signed-off-by: Marcelo Vanzin <[email protected]>
(cherry picked from commit 3d6b44d)
Signed-off-by: Marcelo Vanzin <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Jul 25, 2019
…ty preference into account

Right now, we cancel pending allocate requests by its sending order. I thing we can take

locality preference into account when do this to perfom least impact on task locality preference.

N.A.

Closes apache#23344 from Ngone51/dev-cancel-pending-allocate-requests-by-taking-locality-preference-into-account.

Authored-by: Ngone51 <[email protected]>
Signed-off-by: Marcelo Vanzin <[email protected]>
(cherry picked from commit 3d6b44d)
Signed-off-by: Marcelo Vanzin <[email protected]>
kai-chi pushed a commit to kai-chi/spark that referenced this pull request Aug 1, 2019
…ty preference into account

## What changes were proposed in this pull request?

Right now, we cancel pending allocate requests by its sending order. I thing we can take

locality preference into account when do this to perfom least impact on task locality preference.

## How was this patch tested?

N.A.

Closes apache#23344 from Ngone51/dev-cancel-pending-allocate-requests-by-taking-locality-preference-into-account.

Authored-by: Ngone51 <[email protected]>
Signed-off-by: Marcelo Vanzin <[email protected]>
(cherry picked from commit 3d6b44d)
Signed-off-by: Marcelo Vanzin <[email protected]>
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