[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[AmplabJenkins]

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/19881
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/89467/
Test PASSed.


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[AmplabJenkins]

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/19881
  
Merged build finished. Test PASSed.


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[SparkQA]

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/19881
  
**[Test build #89467 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89467/testReport)**
 for PR 19881 at commit 
[`15732ab`](https://github.com/apache/spark/commit/15732ab7ee22a9cc4409b36812b5b2c930854723).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[rxin]

Github user rxin commented on the issue:

https://github.com/apache/spark/pull/19881
  
Thanks @jcuquemelle 


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[SparkQA]

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/19881
  
**[Test build #89467 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89467/testReport)**
 for PR 19881 at commit 
[`15732ab`](https://github.com/apache/spark/commit/15732ab7ee22a9cc4409b36812b5b2c930854723).


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[jcuquemelle]

Github user jcuquemelle commented on the issue:

https://github.com/apache/spark/pull/19881
  
Ok, will quickly do the change
thanks for the proposals


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[tgravescs]

Github user tgravescs commented on the issue:

https://github.com/apache/spark/pull/19881
  
I'm fine with that


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[rxin]

Github user rxin commented on the issue:

https://github.com/apache/spark/pull/19881
  
I thought about this more, and I actually think something like this makes 
more sense: `executorAllocationRatio`. Basically it is just a ratio that 
determines how aggressive we want Spark to request full executors. Ratio of 1.0 
means fill up everything. Ratio of 0.5 means only request half of the executors.

What do you think?




---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[tgravescs]

Github user tgravescs commented on the issue:

https://github.com/apache/spark/pull/19881
  
No we don't strictly need it in the name, the reasoning behind it was to 
indicate that this was a divisor based on if you have fully allocated executors 
for all the tasks and were running full parallelism. 
Are you suggesting just use 
spark.dynamicAllocation.executorAllocationDivisor?  other ones thrown are were 
like maxExecutorAllocationDivisor.  One thing we were trying to keep from doing 
is confusing it with the maxExecutors config as well.  Opinions?


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[rxin]

Github user rxin commented on the issue:

https://github.com/apache/spark/pull/19881
  
SGTM on divisor.

Do we need "full" there in the config?



---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[tgravescs]

Github user tgravescs commented on the issue:

https://github.com/apache/spark/pull/19881
  
@rxin I assume you are just trying to not use divisor since its not used 
anywhere else?  As @jcuquemelle state I don't see a use case for this to be 
made more aggressive if you have one please let us know, but otherwise it just 
wastes resources. 

Personally I still like divisor because that is what you are doing.  I 
don't think because its not in any other configs is a good reason to not use 
it.   Looking at I don't see any public configs that have factor in the name of 
them either.  I am not fond of rate because its not a rate (ie how 
quickly/slowly you are allocating), its a limit on max number of executors.  

I also think its more natural for people to think of this as a divisor vs a 
multiplier.  if I want 1/2 of the executors you divide by 2.   I think we 
should name it based on what is most likely understood by the end user.



---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[jcuquemelle]

Github user jcuquemelle commented on the issue:

https://github.com/apache/spark/pull/19881
  
more aggressive must be forbidden, because the setting of 1.0 gives enough 
executors so that if the executor provisioning was perfect (e.g. all executors 
were available at the same time) and the mapping of tasks to executors was 
optimal, each executor core (or taskSlot as in the original naming) would 
process exactly one task. If you ask for more executors, you're sure they will 
be wasted.
Which is why it felt natural to have a divisor semantics, because it 
implies the parameter can only be used to reduce parallelism.
How about fullParallelismThrottlingRate ?


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[rxin]

Github user rxin commented on the issue:

https://github.com/apache/spark/pull/19881
  
Maybe instead of "divisor", we just have a "rate" or "factor" that can be 
floating point value, and use multiplication rather than division? This way 
people can also make it even more aggressive.



---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[jcuquemelle]

Github user jcuquemelle commented on the issue:

https://github.com/apache/spark/pull/19881
  
@rxin , can we merge this PR ?


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[tgravescs]

Github user tgravescs commented on the issue:

https://github.com/apache/spark/pull/19881
  
Yes we can wait another day or so if you are looking at it, this discussion 
has been going on for a long time now though, if you have a better name 
suggestion let us know.  No other configs have "divisor" suffix.s 


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[rxin]

Github user rxin commented on the issue:

https://github.com/apache/spark/pull/19881
  
Can you wait another day? I just find the name pretty weird. Do we have
other configs that use the “divisor” suffix?

On Wed, Mar 28, 2018 at 7:23 AM Tom Graves  wrote:

> I'll leave this a bit longer but then I'm going to merge it later today
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> , or 
mute
> the thread
> 

> .
>



---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[tgravescs]

Github user tgravescs commented on the issue:

https://github.com/apache/spark/pull/19881
  
I'll leave this a bit longer but then I'm going to merge it later today


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[jiangxb1987]

Github user jiangxb1987 commented on the issue:

https://github.com/apache/spark/pull/19881
  
cc @rxin


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[tgravescs]

Github user tgravescs commented on the issue:

https://github.com/apache/spark/pull/19881
  
+1


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[tgravescs]

Github user tgravescs commented on the issue:

https://github.com/apache/spark/pull/19881
  
just minor comment about the test otherwise looks good.


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[AmplabJenkins]

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/19881
  
Merged build finished. Test PASSed.


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[AmplabJenkins]

Github user AmplabJenkins commented on the issue:

https://github.com/apache/spark/pull/19881
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/88472/
Test PASSed.


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[SparkQA]

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/19881
  
**[Test build #88472 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88472/testReport)**
 for PR 19881 at commit 
[`a40d160`](https://github.com/apache/spark/commit/a40d16031a4618db4023a3cf432b4d8f37f70b7d).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds no public classes.


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[SparkQA]

Github user SparkQA commented on the issue:

https://github.com/apache/spark/pull/19881
  
**[Test build #88472 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88472/testReport)**
 for PR 19881 at commit 
[`a40d160`](https://github.com/apache/spark/commit/a40d16031a4618db4023a3cf432b4d8f37f70b7d).


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[tgravescs]

Github user tgravescs commented on the issue:

https://github.com/apache/spark/pull/19881
  
jenkins, ok to test


---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[tgravescs]

Github user tgravescs commented on the issue:

https://github.com/apache/spark/pull/19881
  
jenkins, test this please





---

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

[GitHub] spark issue #19881: [SPARK-22683][CORE] Add a fullExecutorAllocationDivisor ...

[jcuquemelle]

Github user jcuquemelle commented on the issue:

https://github.com/apache/spark/pull/19881
  
@felixcheung: updated PR title and description


---

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