Skip to content

[SPARK-13958]Executor OOM due to unbounded growth of pointer array in… #11794

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.

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

Closed
wants to merge 1 commit into from

Conversation

sitalkedia
Copy link

What changes were proposed in this pull request?

This change fixes the executor OOM which was recently introduced in PR #11095
(Please fill in changes proposed in this fix)

How was this patch tested?

Tested by running a spark job on the cluster.
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

… Sorter

@srowen
Copy link
Member

srowen commented Mar 18, 2016

CC @davies

array = allocateArray(used / 8 * 2);
} catch (OutOfMemoryError e) {
// should have trigger spilling
assert(inMemSorter.hasSpaceForAnotherRecord());
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you change this to require and add a error message?

Copy link
Author

Choose a reason for hiding this comment

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

Hmm.. I tried changing it to require but compiler does not seem to like it. May be because its a java file and can't import scala methods?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, then use a if ?

Copy link
Author

Choose a reason for hiding this comment

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

Done.

@davies
Copy link
Contributor

davies commented Mar 18, 2016

Jenkins, ok to test.

@davies
Copy link
Contributor

davies commented Mar 18, 2016

LGTM

@SparkQA
Copy link

SparkQA commented Mar 18, 2016

Test build #53543 has finished for PR 11794 at commit 108edc8.

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

@davies
Copy link
Contributor

davies commented Mar 18, 2016

Merging this into master and 1.6, thanks!

asfgit pushed a commit that referenced this pull request Mar 18, 2016
## What changes were proposed in this pull request?

This change fixes the executor OOM which was recently introduced in PR #11095
(Please fill in changes proposed in this fix)

## How was this patch tested?
Tested by running a spark job on the cluster.
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

… Sorter

Author: Sital Kedia <skedia@fb.com>

Closes #11794 from sitalkedia/SPARK-13958.

(cherry picked from commit 2e0c528)
Signed-off-by: Davies Liu <davies.liu@gmail.com>
@asfgit asfgit closed this in 2e0c528 Mar 18, 2016
@SparkQA
Copy link

SparkQA commented Mar 18, 2016

Test build #53550 has finished for PR 11794 at commit 4db3880.

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

roygao94 pushed a commit to roygao94/spark that referenced this pull request Mar 22, 2016
## What changes were proposed in this pull request?

This change fixes the executor OOM which was recently introduced in PR apache#11095
(Please fill in changes proposed in this fix)

## How was this patch tested?
Tested by running a spark job on the cluster.
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

… Sorter

Author: Sital Kedia <skedia@fb.com>

Closes apache#11794 from sitalkedia/SPARK-13958.
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.

4 participants