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.

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

Bug fix for SPARK-5242: "ec2/spark_ec2.py lauch" does not work with VPC if no public DNS or IP is available #4038

Closed
wants to merge 3 commits into from

Conversation

voukka
Copy link

@voukka voukka commented Jan 14, 2015

The fix for https://issues.apache.org/jira/browse/SPARK-5242

This pull request is my original work and I contribute it under project's open source license.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@andrewor14
Copy link
Contributor

ok to test. @nchammas

@SparkQA
Copy link

SparkQA commented Jan 15, 2015

Test build #25585 has started for PR 4038 at commit c396062.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Jan 15, 2015

Test build #25585 has finished for PR 4038 at commit c396062.

  • 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/25585/
Test PASSed.

@nchammas
Copy link
Contributor

Reported bug confirmed. Now testing fix.

@nchammas
Copy link
Contributor

Hmm, it seems that setting up an environment to test this fix is non-trivial.

@voukka Could you share a minimal VPC and subnet setup that I can duplicate to test this fix? Is it something similar to the scenario described here?

@voukka
Copy link
Author

voukka commented Jan 15, 2015

@nchammas yes, http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_Scenario2.html should be sufficient to test this bug. After you followed that guide, start new instance in VPC, ssh to it (though NAT server) and launch cluster as in https://issues.apache.org/jira/browse/SPARK-5242.

The setup where I strive to launch the cluster and found this bug is closer to http://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_Scenario3.html, which differs from Scenario 2 by VPN connection to corporate network.

@nchammas
Copy link
Contributor

cc @shivaram

I haven't had a chance to look at this more closely yet, and likely won't until next weekend.

@shivaram
Copy link
Contributor

@voukka @nchammas - This high level goal looks fine to me. However I the function get_hostname is being called on all instances (its inside a loop) in many cases. I wonder if we can do something more lightweight by exploiting the fact that you typically want to use the same kind of resolution for all machines. What this will mean is that for the very first machine we will try all four options and then just save which field was used -- Then the function just picks the appropriate field going forward.

Will this solve your use case ? Or are there use cases where we need to do this for every instance ?

@srowen
Copy link
Member

srowen commented Feb 15, 2015

@voukka Is this change still live? I see @shivaram has some outstanding concerns.

@voukka
Copy link
Author

voukka commented Feb 16, 2015

@shivaram that's good point to store selected address type.
@srowen I think it requires a bit more testing. Hopefully I'll get some time for that today

@pishen
Copy link

pishen commented Mar 16, 2015

Is there any plan to merge this? We also need this feature.

@nchammas
Copy link
Contributor

@pishen This patch has a merge conflict and needs updating + more testing per @voukka's previous comment.

@voukka
Copy link
Author

voukka commented Mar 18, 2015

I will try to update/resolve conflicts today/tomorrow.

@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/29705/
Test PASSed.

@voukka
Copy link
Author

voukka commented Apr 7, 2015

Superseded by #5244

@voukka voukka closed this Apr 7, 2015
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.

8 participants