[SPARK-5668] Display region in spark_ec2.py get_existing_cluster() #4457

MiguelPeralvo · 2015-02-08T10:39:05Z

Show the region for the different messages displayed by get_existing_cluster(): The search, found and error messages.

AmplabJenkins · 2015-02-08T10:42:09Z

Can one of the admins verify this patch?

srowen · 2015-02-08T11:13:51Z

ok to test

SparkQA · 2015-02-08T11:19:15Z

Test build #27039 has started for PR 4457 at commit 4ecd9f9.

This patch merges cleanly.

SparkQA · 2015-02-08T12:39:37Z

Test build #27039 has finished for PR 4457 at commit 4ecd9f9.

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

AmplabJenkins · 2015-02-08T12:39:41Z

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

nchammas · 2015-02-08T22:34:47Z

ec2/spark_ec2.py

@@ -607,14 +608,17 @@ def get_existing_cluster(conn, opts, cluster_name, die_on_error=True):
            elif (cluster_name + "-slaves") in group_names:
                slave_nodes.append(inst)
    if any((master_nodes, slave_nodes)):
-        print "Found %d master(s), %d slaves" % (len(master_nodes), len(slave_nodes))
+        print "Found %d master(s), %d slaves" % (len(master_nodes), len(slave_nodes)) \
+            + " in region " + opts.region


Really minor point, but I would leave this print statement unchanged since users are unlikely to take action at this particular point, and we printed the region earlier.

@nchmmas, I was wondering the same. I'll remove it when I review it. Thank you.

nchammas · 2015-02-08T22:37:00Z

@MiguelPeralvo Do you think we should print the region when launching a cluster, or does that already happen?

Also, I think we should update the help text for the --region option to indicate that this is the region used to find instances, not just launch them.

MiguelPeralvo · 2015-02-09T16:13:44Z

@nchammas,

Regarding the help text, I'd say that it applies to the destroy, login, reboot-slaves, get-master, stop and start options, not only launch. Something like "EC2 region to launch/destroy/login/reboot-slaves/get-master/stop/start instances in" or "EC2 region where to apply action to: launch/destroy/login/reboot-slaves/get-master/stop/start instances in" is a potential text, but I'm happy to apply any other that reflects better where the parameter region applies, maybe "EC2 region used to find instances in or to launch them in", as you proposed (all the other actions involve the search).

Regarding the region being printed when launching a cluster: As the "get_existing_cluster()" method gets used by all actions, the region will get printed for all of them if the proposed change is applied. I'm in favor of the region being printed out. For example, this would be the beginning of the output of a launch execution with the region change being applied:

Miguels-MacBook-Pro:ec2 Miguel$ $SPARK_HOME/ec2/spark-ec2 -k xyzapachespark -i $SSH_HOME/xyzapachespark.pem -r eu-west-1 launch my-spark-cluster

Setting up security groups...
Searching for existing cluster my-spark-cluster in region eu-west-1...
Spark AMI: ami-1ae0166d
Launching instances...
Launched 1 slaves in eu-west-1c, regid = r-679b8425
Launched master in eu-west-1c, regid = r-e69d82a4
...

If we don't apply the proposed change to the "get_existing_cluster()" method, the zone appears, but not the region. For example, this would be the beginning of the output of a launch execution:

Setting up security groups...
Searching for existing cluster my-spark-cluster ...
Spark AMI: ami-1ae0166d
Launching instances...
Launched 1 slaves in eu-west-1c, regid = r-679b8425
Launched master in eu-west-1c, regid = r-e69d82a4
...
Warning: Permanently added 'ec2-54-194-24-152.eu-west-1.compute.amazonaws.com,54.194.24.152' (RSA) to the list of known hosts.

I've tested launch with the default region, without the region change and when the key pair is not found in the default region, this is the output I get from spark-ec2 (again, it makes it a little bit difficult for a new user to figure out what's wrong):

Setting up security groups...
Searching for existing cluster my-spark-cluster...
Spark AMI: ami-5bb18832
Launching instances...
ERROR:boto:400 Bad Request
ERROR:boto:<?xml version="1.0" encoding="UTF-8"?>
<Response><Errors><Error><Code>InvalidKeyPair.NotFound</Code><Message>The key pair 'xyzapachespark' does not exist</Message></Error></Errors><RequestID>7ef88166-5cee-4c2a-acdc-f77ebe34acfc</RequestID></Response>
Traceback (most recent call last):
...

Please, feel free to change the wording for --region, if required.

SparkQA · 2015-02-10T09:17:27Z

Test build #27191 has started for PR 4457 at commit 3923f36.

This patch merges cleanly.

SparkQA · 2015-02-10T09:18:26Z

Test build #27191 has finished for PR 4457 at commit 3923f36.

This patch fails Python style tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-02-10T09:18:27Z

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

Removed trailing space from line 611 to make it PEP 8 compliant.

SparkQA · 2015-02-10T09:27:30Z

Test build #27192 has started for PR 4457 at commit 0a837b0.

This patch merges cleanly.

SparkQA · 2015-02-10T10:38:01Z

Test build #27192 has finished for PR 4457 at commit 0a837b0.

This patch fails MiMa tests.
This patch merges cleanly.
This patch adds no public classes.

AmplabJenkins · 2015-02-10T10:38:04Z

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

MiguelPeralvo · 2015-02-10T16:17:43Z

The MiMa fail may be flaky. It passed in my computer.
Jenkins, retest this please.

srowen · 2015-02-10T16:34:41Z

ec2/spark_ec2.py

@@ -132,7 +132,7 @@ def parse_args():
        help="Master instance type (leave empty for same as instance-type)")
    parser.add_option(
        "-r", "--region", default="us-east-1",
-        help="EC2 region zone to launch instances in")
+        help="EC2 region used to to launch instances in, or to find them in")


Nit: "to to". Yes the Mima result must be a false positive given the nature of this change.

@srowen

Fixed typo found by @srowen "to to".

SparkQA · 2015-02-10T16:52:27Z

Test build #27211 has started for PR 4457 at commit a5514c8.

This patch merges cleanly.

SparkQA · 2015-02-10T18:05:35Z

Test build #27211 has finished for PR 4457 at commit a5514c8.

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

AmplabJenkins · 2015-02-10T18:05:39Z

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

srowen · 2015-02-10T18:31:51Z

Looks good. If there aren't any additional comments, I can merge this soon

nchammas · 2015-02-10T19:37:32Z

LGTM

[SPARK-5668] Display region in spark_ec2.py get_existing_cluster()

4ecd9f9

Show the region for the different messages displayed by get_existing_cluster(): The search, found and error messages.

nchammas reviewed Feb 8, 2015
View reviewed changes

Update spark_ec2.py

3923f36

Please, feel free to change the wording for --region, if required.

Update spark_ec2.py

0a837b0

Removed trailing space from line 611 to make it PEP 8 compliant.

srowen reviewed Feb 10, 2015
View reviewed changes

Update spark_ec2.py

a5514c8

Fixed typo found by @srowen "to to".

asfgit closed this in c49a404 Feb 10, 2015

MiguelPeralvo deleted the patch-2 branch February 10, 2015 20:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-5668] Display region in spark_ec2.py get_existing_cluster() #4457

[SPARK-5668] Display region in spark_ec2.py get_existing_cluster() #4457

MiguelPeralvo commented Feb 8, 2015

AmplabJenkins commented Feb 8, 2015

srowen commented Feb 8, 2015

SparkQA commented Feb 8, 2015

SparkQA commented Feb 8, 2015

AmplabJenkins commented Feb 8, 2015

nchammas Feb 8, 2015

MiguelPeralvo Feb 9, 2015

nchammas commented Feb 8, 2015

MiguelPeralvo commented Feb 9, 2015

SparkQA commented Feb 10, 2015

SparkQA commented Feb 10, 2015

AmplabJenkins commented Feb 10, 2015

SparkQA commented Feb 10, 2015

SparkQA commented Feb 10, 2015

AmplabJenkins commented Feb 10, 2015

MiguelPeralvo commented Feb 10, 2015

srowen Feb 10, 2015

SparkQA commented Feb 10, 2015

SparkQA commented Feb 10, 2015

AmplabJenkins commented Feb 10, 2015

srowen commented Feb 10, 2015

nchammas commented Feb 10, 2015

[SPARK-5668] Display region in spark_ec2.py get_existing_cluster() #4457

[SPARK-5668] Display region in spark_ec2.py get_existing_cluster() #4457

Conversation

MiguelPeralvo commented Feb 8, 2015

AmplabJenkins commented Feb 8, 2015

srowen commented Feb 8, 2015

SparkQA commented Feb 8, 2015

SparkQA commented Feb 8, 2015

AmplabJenkins commented Feb 8, 2015

nchammas Feb 8, 2015

Choose a reason for hiding this comment

MiguelPeralvo Feb 9, 2015

Choose a reason for hiding this comment

nchammas commented Feb 8, 2015

MiguelPeralvo commented Feb 9, 2015

SparkQA commented Feb 10, 2015

SparkQA commented Feb 10, 2015

AmplabJenkins commented Feb 10, 2015

SparkQA commented Feb 10, 2015

SparkQA commented Feb 10, 2015

AmplabJenkins commented Feb 10, 2015

MiguelPeralvo commented Feb 10, 2015

srowen Feb 10, 2015

Choose a reason for hiding this comment

SparkQA commented Feb 10, 2015

SparkQA commented Feb 10, 2015

AmplabJenkins commented Feb 10, 2015

srowen commented Feb 10, 2015

nchammas commented Feb 10, 2015