[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user srowen commented on the issue: https://github.com/apache/spark/pull/19183 I thought this check also existed in the non-streaming code; the theory was that if you have set a fixed number of executors but enabled dynamic allocation, then that's probably a configuration error. But given that many people run on clusters with dynamic allocation defaulting to 'on' globally, that could be confusing or a little inconvenient to work around. I don't think that check exists in the non-streaming code anymore though, and I see a test to that effect too. Therefore I think this is reasonable for consistency. CC @tdas --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19183 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user skonto commented on the issue: https://github.com/apache/spark/pull/19183 @karth295 I think you could validate the config [here](https://github.com/apache/spark/blob/fa2ae9d2019f839647d17932d8fea769e7622777/core/src/main/scala/org/apache/spark/SparkContext.scala#L365), in the validateSettings method for SparkConfig. I guess this is where you have all the info required. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user karth295 commented on the issue: https://github.com/apache/spark/pull/19183 @sansagara go for it -- it'll be a few days until I'll have time to look at this again. I'll close my PR if/when you make a new one :) --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user koeninger commented on the issue: https://github.com/apache/spark/pull/19183 @sansagara sounds reasonable to me --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user sansagara commented on the issue: https://github.com/apache/spark/pull/19183 I can add a test on ExecutorAllocationManagerSuite.scala to assert spark.executor.instances correctly allocates that number of executors initially when uaing Streaming DA --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user koeninger commented on the issue: https://github.com/apache/spark/pull/19183 I don't have personal experience with streaming dynamic allocation, but this patch makes sense to me and I don't see anything obviously wrong. I agree with Holden regarding tests. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19183 **[Test build #90232 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90232/testReport)** for PR 19183 at commit [`4c9769e`](https://github.com/apache/spark/commit/4c9769e232a5d028f48235260bde682a1d3b059a). * 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 #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19183 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90232/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19183 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 #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user holdenk commented on the issue: https://github.com/apache/spark/pull/19183 Also one point, the lack of tests leaves me a little concerned about this change, maybe look at `./streaming/src/test/scala/org/apache/spark/streaming/scheduler/ExecutorAllocationManagerSuite.scala` and see if it would make sense to add something there? This is a bit out of my usual range so I'm going to CC @koeninger to take a more detailed look. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user holdenk commented on the issue: https://github.com/apache/spark/pull/19183 @karth295 for validating that spark.executors is in a valid range could we look at it in the config with ConfigBuilder checkValue ? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19183 **[Test build #90232 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90232/testReport)** for PR 19183 at commit [`4c9769e`](https://github.com/apache/spark/commit/4c9769e232a5d028f48235260bde682a1d3b059a). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user holdenk commented on the issue: https://github.com/apache/spark/pull/19183 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 #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user sansagara commented on the issue: https://github.com/apache/spark/pull/19183 Just commenting to be subscribed! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user sansagara2 commented on the issue: https://github.com/apache/spark/pull/19183 This is actually a good design solution. Right now, it is not very clear (not even docs, examples, google searches) how to set an initial number of executors for a Streaming Application that haves StreamingDynamicAllocation enabled. I considerer this important, cause for some streams, like Kinesis... a minimun number of executors are needed, to match shards. Any quick workaround for this? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19183 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19183 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19183 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19183: [SPARK-21960][Streaming] Spark Streaming Dynamic Allocat...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19183 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
