Skip to content

Conversation

sryza
Copy link
Contributor

@sryza sryza commented Dec 10, 2014

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should go after java and scala imports.

@SparkQA
Copy link

SparkQA commented Dec 10, 2014

Test build #24273 has started for PR 3652 at commit 17628f8.

  • This patch merges cleanly.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should be right after java imports.

@vanzin
Copy link
Contributor

vanzin commented Dec 10, 2014

I assume most here is just code motion, so I didn't pay that close attention to the changes in the container allocation code. If that's true, then this LGTM.

@SparkQA
Copy link

SparkQA commented Dec 10, 2014

Test build #24273 has finished for PR 3652 at commit 17628f8.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24273/
Test PASSed.

Copy link
Contributor

Choose a reason for hiding this comment

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

delete

@andrewor14
Copy link
Contributor

This looks fine on a first glance. I plan to take a detailed pass in the next day or two to make sure we didn't change any logic.

@andrewor14
Copy link
Contributor

Just took a deeper look comparing the old code with the new code and couldn't find any glaring problems. I'll test and merge this once you address the comments.

@sryza
Copy link
Contributor Author

sryza commented Dec 20, 2014

Cool, updated patch reflects Marcelo and Andrew's comments.

@SparkQA
Copy link

SparkQA commented Dec 20, 2014

Test build #24676 has started for PR 3652 at commit 2791158.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Dec 20, 2014

Test build #24676 has finished for PR 3652 at commit 2791158.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24676/
Test PASSed.

@andrewor14
Copy link
Contributor

Cool I just tested this on a Hadoop 2.4 cluster and it worked like a charm. I'm merging this into master thanks @sryza @vanzin.

@asfgit asfgit closed this in d62da64 Dec 22, 2014
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