You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by vidaha <gi...@git.apache.org> on 2014/08/12 04:01:51 UTC

[GitHub] spark pull request: Vida/ec2 reuse security group

GitHub user vidaha opened a pull request:

    https://github.com/apache/spark/pull/1899

    Vida/ec2 reuse security group

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/vidaha/spark vida/ec2-reuse-security-group

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/1899.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1899
    
----
commit 62e1cd2af0c99697b62791226c67e4bc99c9397e
Author: Vida Ha <vi...@databricks.com>
Date:   2014-08-12T01:06:51Z

    Add an option to reuse security groups

commit bce4182065a815fdd79d3bf8d6f2ada84c8cec9e
Author: Vida Ha <vi...@databricks.com>
Date:   2014-08-12T01:57:54Z

    Allow reusing a security group prefix rather than always having to create a new one for each cluster

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: Vida/ec2 reuse security group

Posted by vidaha <gi...@git.apache.org>.
Github user vidaha commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-51949860
  
    Hi Josh,
    
    IMHO, it's best not to require a Spark cluster name and the security group to be the same.  While you can reuse an existing security group to launch another cluster, you can't launch more than one cluster with the same security group.  Perhaps a company wants to have an internal-applications or dev security group and reuse that for launch multiple Spark clusters.  In addition, AWS has a strict limit of 100 on the number of security groups on a VPC, and since two security groups are required (one of the masters and one for the workers), this means, that only 50 Spark cluster can be launched on a VPC.  While that might seem like a reasonable limit, I can easily see companies having a use case to exceed that.
    
    Do you mind illustrating the problem about name conflicts?  If I understand what you are saying, you are mentioned this scenario:
    
    % ./spark-ec2 … —security-group my-security-group launch my-cluster-name
    
    And then later, you also run:
    
    % ./spark-ec2 … launch my-security-group
    
    This works fine - I tested it - there will be two clusters with the same security group, but different names.  These are some error cases that I thought might offer and tested for manually and they worked out fine:
    
    - Can you then run ./spark-ec2 … —delete-groups destroy my-security-group and delete the security group when another cluster is using that security group?
    
    I tried this, and amazon has the correct controls to prevent deleting a security group still in use by another cluster.
    
    - Can you forget to include the security group override on a launched cluster and create problems?
    
    I tried this as well, and since the get_existing_cluster code was modified to use the name rather than the security group to identify the instance, this works right.  You can’t run:
    
    % ./spark-ec2 … launch my-cluster-name
    
    if there is already a cluster with my-cluster-name launched.
    
    
    Are there some other possible conflicts that you can think of?  If you can write out the commands to illustrate the use cases you are thinking of - I can run them and see what happens.
    
    -Vida


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:

    https://github.com/apache/spark/pull/1899#discussion_r16197565
  
    --- Diff: ec2/spark_ec2.py ---
    @@ -382,8 +391,8 @@ def launch_cluster(conn, opts, cluster_name):
                             slave_nodes += r.instances
                         break
                     else:
    -                    print "%d of %d slaves granted, waiting longer" % (
    -                        len(active_instance_ids), opts.slaves)
    +                    print "%d of %d slaves granted, waiting longer for request ids %s" % (
    +                        len(active_instance_ids), opts.slaves, outstanding_request_ids)
    --- End diff --
    
    Not sure this is related to this PR. Also when launching large clusters (say 100 or 200 spot instances) then this will print all the request ids and overflow lines in the console output ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-51955708
  
    QA tests have started for PR 1899. This patch merges cleanly. <br>View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18376/consoleFull


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - spark_ec2 script should allow opt...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/1899


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52135326
  
    QA results for PR 1899:<br>- This patch PASSES unit tests.<br>- This patch merges cleanly<br>- This patch adds no public classes<br><br>For more information see test ouptut:<br>https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18505/consoleFull


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52134173
  
    @vidaha please update the PR title and description to say what the change does. Those actually become the final commit message in git.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: Vida/ec2 reuse security group

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-51953660
  
    After a closer look, it looks like the existing `spark-ec2` already names instances after `cluster_name`, so it's safe and backwards-compatible to use that to identify existing clusters.
    
    The code in this PR looks good; a couple of TODOs:
    
    - Update the pull request title to reference a new or existing JIRA.  I think [SPARK-2333](https://issues.apache.org/jira/browse/SPARK-2333) fits the bill, so I'd name it something like "[SPARK-2333] Allow multiple spark-ec2 clusters to be launched in the same security group".
    - Maybe update the intro of the [Running Spark on EC2 guide](http://spark.apache.org/docs/latest/ec2-scripts.html) guide to explain that the clusters are identified by their instances' names instead of security groups.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - spark_ec2 script should allow opt...

Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52686462
  
    LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by vidaha <gi...@git.apache.org>.
Github user vidaha commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52134043
  
    Great - I made the small edits, and adding a loop for retrying the tagging logic.  I tested to see if I can bring up a cluster, and it went up fine - it didn't try tagging more than once.  I have no way of testing the tagging retry logic though.
    
    By the way, I'm amending my commits rather than create a new one per edit - is that standard Spark etiquette?  Let me know if not. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - spark_ec2 script should allow opt...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/1899#discussion_r17812734
  
    --- Diff: ec2/spark_ec2.py ---
    @@ -440,14 +449,29 @@ def launch_cluster(conn, opts, cluster_name):
             print "Launched master in %s, regid = %s" % (zone, master_res.id)
     
         # Give the instances descriptive names
    +    # TODO: Add retry logic for tagging with name since it's used to identify a cluster.
         for master in master_nodes:
    -        master.add_tag(
    -            key='Name',
    -            value='{cn}-master-{iid}'.format(cn=cluster_name, iid=master.id))
    +        name = '{cn}-master-{iid}'.format(cn=cluster_name, iid=master.id)
    +        for i in range(0, 5):
    +            try:
    +                master.add_tag(key='Name', value=name)
    --- End diff --
    
    note that we need a break here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:

    https://github.com/apache/spark/pull/1899#discussion_r16197440
  
    --- Diff: ec2/spark_ec2.py ---
    @@ -138,7 +138,9 @@ def parse_args():
         parser.add_option(
             "--user-data", type="string", default="",
             help="Path to a user-data file (most AMI's interpret this as an initialization script)")
    -
    +    parser.add_option(
    +        "--security-group-prefix", type="string", default=None,
    +        help="Use this prefix for teh security group rather than the cluster name.")
    --- End diff --
    
    Typo: teh -> the


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - spark_ec2 script should allow opt...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/1899#discussion_r17812732
  
    --- Diff: ec2/spark_ec2.py ---
    @@ -440,14 +449,29 @@ def launch_cluster(conn, opts, cluster_name):
             print "Launched master in %s, regid = %s" % (zone, master_res.id)
     
         # Give the instances descriptive names
    +    # TODO: Add retry logic for tagging with name since it's used to identify a cluster.
         for master in master_nodes:
    -        master.add_tag(
    -            key='Name',
    -            value='{cn}-master-{iid}'.format(cn=cluster_name, iid=master.id))
    +        name = '{cn}-master-{iid}'.format(cn=cluster_name, iid=master.id)
    +        for i in range(0, 5):
    +            try:
    +                master.add_tag(key='Name', value=name)
    +            except:
    +                print "Failed attempt %i of 5 to tag %s" % ((i + 1), name)
    +                if (i == 5):
    +                    raise "Error - failed max attempts to add name tag"
    +                time.sleep(5)
    +
    +
         for slave in slave_nodes:
    -        slave.add_tag(
    -            key='Name',
    -            value='{cn}-slave-{iid}'.format(cn=cluster_name, iid=slave.id))
    +        name = '{cn}-slave-{iid}'.format(cn=cluster_name, iid=slave.id)
    +        for i in range(0, 5):
    +            try:
    +                slave.add_tag(key='Name', value=name)
    --- End diff --
    
    note that we need a break here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-51954787
  
    Jenkins, this is ok to test.  Test this please.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - spark_ec2 script should allow opt...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52576660
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18799/consoleFull) for   PR 1899 at commit [`c80d5c3`](https://github.com/apache/spark/commit/c80d5c35c43b6596c2a745bf10f6a751321e87ae).
     * This patch merges cleanly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - Allow option to specify/reuse a s...

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52335044
  
    The committers use the [merge_spark_pr.py](https://github.com/apache/spark/blob/master/dev/merge_spark_pr.py) script for merging pull requests.  This script will squash together all of your commits into a single commit that uses the title of the PR plus the PR's description as the commit message, so you no longer need to worry about rebasing your patch into a single commit.
    
    I'd just copy-paste the description from your last commit into the description above so it becomes the commit message


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - spark_ec2 script should allow opt...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52580052
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18799/consoleFull) for   PR 1899 at commit [`c80d5c3`](https://github.com/apache/spark/commit/c80d5c35c43b6596c2a745bf10f6a751321e87ae).
     * This patch **passes** unit tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52095346
  
    Some minor comments, but otherwise LGTM.
    
    One thing that might be worth noting is that we did use tags while launching clusters for AMPCamp[1] and one issue I ran into was that sometimes the create tag command failed because of eventual consistency between the instance being allocated and the instance being tag-able.
    
    We might want to retry the tag operation a few times since it is the only way to identify instances now.
    
    [1] https://github.com/amplab/training-scripts/blob/ampcamp4/spark_ec2.py#L342


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - spark_ec2 script should allow opt...

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52685205
  
    @shivaram @pwendell This looks good to me.  Does one of you want to take a final look + signoff? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: Vida/ec2 reuse security group

Posted by vidaha <gi...@git.apache.org>.
Github user vidaha commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-51865214
  
    Just FYI - I tested this and it worked - not sure if it's controversial to use the name tag rather than the group for destroy a cluster.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - Allow option to specify/reuse a s...

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/1899#discussion_r16304946
  
    --- Diff: ec2/spark_ec2.py ---
    @@ -440,14 +449,22 @@ def launch_cluster(conn, opts, cluster_name):
             print "Launched master in %s, regid = %s" % (zone, master_res.id)
     
         # Give the instances descriptive names
    +    # TODO: Add retry logic for tagging with name since it's used to identify a cluster.
         for master in master_nodes:
    -        master.add_tag(
    -            key='Name',
    -            value='{cn}-master-{iid}'.format(cn=cluster_name, iid=master.id))
    +        name = '{cn}-master-{iid}'.format(cn=cluster_name, iid=master.id)
    +        for i in range(0, 5):
    +            master.add_tag(key='Name', value=name)
    --- End diff --
    
    What happens if an `add_tag` call fails?  My bet is that it throws an exception rather than silently failing, in which case this re-try logic won't run.  Rather than using this "set-and-test" logic, maybe we can just wrap the call in a try-except block?
    
    @shivaram Did the eventual-consistency issue that you saw result in exceptions from `add_tag`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - Allow option to specify/reuse a s...

Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:

    https://github.com/apache/spark/pull/1899#discussion_r16305293
  
    --- Diff: ec2/spark_ec2.py ---
    @@ -440,14 +449,22 @@ def launch_cluster(conn, opts, cluster_name):
             print "Launched master in %s, regid = %s" % (zone, master_res.id)
     
         # Give the instances descriptive names
    +    # TODO: Add retry logic for tagging with name since it's used to identify a cluster.
         for master in master_nodes:
    -        master.add_tag(
    -            key='Name',
    -            value='{cn}-master-{iid}'.format(cn=cluster_name, iid=master.id))
    +        name = '{cn}-master-{iid}'.format(cn=cluster_name, iid=master.id)
    +        for i in range(0, 5):
    +            master.add_tag(key='Name', value=name)
    --- End diff --
    
    Yes - I am pretty sure it throws an exception. I don't remember what the type is -- All I see in my notes is that the exception says 'Instance not found'


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52134348
  
    QA tests have started for PR 1899. This patch merges cleanly. <br>View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18510/consoleFull


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - spark_ec2 script should allow opt...

Posted by douglaz <gi...@git.apache.org>.
Github user douglaz commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-54001048
  
    Opened an issue related to this PR: https://issues.apache.org/jira/browse/SPARK-3180


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-51965003
  
    QA results for PR 1899:<br>- This patch PASSES unit tests.<br>- This patch merges cleanly<br>- This patch adds no public classes<br><br>For more information see test ouptut:<br>https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18379/consoleFull


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - spark_ec2 script should allow opt...

Posted by vidaha <gi...@git.apache.org>.
Github user vidaha commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52576663
  
    Okay, I made the retry a try catch, and edited the title again.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: Vida/ec2 reuse security group

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-51867038
  
    Why is it useful to have the cluster name be different from the security group prefix?  If I want to re-use an existing security group, I can just name my cluster after that security group.  The cluster name only seems to be used to name the security groups, instance requests, and instances.
    
    What would happen if I launched a cluster with a name that's different from its security group, then attempted to run spark-ec2 commands by passing only the actual security group name as the cluster name?  In this case, I think the instances would still be named with the cluster name, which might cause problems since the script would be expecting to find ones named after the security group.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: Vida/ec2 reuse security group

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-51865131
  
    Can one of the admins verify this patch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-51957963
  
    QA tests have started for PR 1899. This patch merges cleanly. <br>View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18379/consoleFull


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52137075
  
    QA results for PR 1899:<br>- This patch PASSES unit tests.<br>- This patch merges cleanly<br>- This patch adds no public classes<br><br>For more information see test ouptut:<br>https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18510/consoleFull


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52132336
  
    QA tests have started for PR 1899. This patch merges cleanly. <br>View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18505/consoleFull


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52093012
  
    This looks good to me.  @pwendell or @shivaram, do you want to take a look at this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-51963248
  
    QA results for PR 1899:<br>- This patch PASSES unit tests.<br>- This patch merges cleanly<br>- This patch adds no public classes<br><br>For more information see test ouptut:<br>https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18376/consoleFull


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - spark_ec2 script should allow opt...

Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:

    https://github.com/apache/spark/pull/1899#discussion_r16437659
  
    --- Diff: ec2/spark_ec2.py ---
    @@ -440,14 +449,29 @@ def launch_cluster(conn, opts, cluster_name):
             print "Launched master in %s, regid = %s" % (zone, master_res.id)
     
         # Give the instances descriptive names
    +    # TODO: Add retry logic for tagging with name since it's used to identify a cluster.
    --- End diff --
    
    Minor nit: You can remove this TODO now. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-2333 - spark_ec2 script should allow opt...

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/1899#issuecomment-52694532
  
    Alright, great.  I'm going to merge this into `master` and `branch-1.1`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org