You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by ArcherShao <gi...@git.apache.org> on 2015/03/21 09:08:38 UTC

[GitHub] spark pull request: [SPARK-5961][Streaming]Allow specific nodes in...

GitHub user ArcherShao opened a pull request:

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

    [SPARK-5961][Streaming]Allow specific nodes in a Spark Streaming cluster to be preferred as Receiver Worker Nodes

    1.The function ’setPreferredLocation‘ is in  class ReceiverInputDStream.
    2.FlumeInputDStream take data send node as the defalut PreferredLocation

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

    $ git pull https://github.com/ArcherShao/spark receiver-preferred-location

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

    https://github.com/apache/spark/pull/5114.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 #5114
    
----
commit 324d103180ec2e2bc2824da4fb5efaf4579db098
Author: ArcherShao <sh...@huawei.com>
Date:   2015-03-21T07:08:34Z

    Allow specific nodes in a Spark Streaming cluster to be dedicated/preferred as Receiver Worker Nodes versus regular Spark Worker Nodes

commit f189b08514c73bdc7de802324affd913b3ef474f
Author: ArcherShao <sh...@huawei.com>
Date:   2015-03-21T07:34:14Z

    update

----


---
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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#issuecomment-84278502
  
      [Test build #28941 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28941/consoleFull) for   PR 5114 at commit [`f189b08`](https://github.com/apache/spark/commit/f189b08514c73bdc7de802324affd913b3ef474f).
     * 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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#discussion_r26922663
  
    --- Diff: streaming/src/main/scala/org/apache/spark/streaming/receiver/Receiver.scala ---
    @@ -107,9 +107,6 @@ abstract class Receiver[T](val storageLevel: StorageLevel) extends Serializable
        */
       def onStop()
     
    -  /** Override this to specify a preferred location (hostname). */
    -  def preferredLocation : Option[String] = None
    --- End diff --
    
    I have tried, but didn't find a good way to pass the argument through `ReceiverInputDStream` to this by adding some methods.
    IMO, `ReceiverInputDStream` is also an interface for user, user can overide preferredLocation (`ReceiverInputDStream`) in their own implementation.


---
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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#issuecomment-96769323
  
      [Test build #31048 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31048/consoleFull) for   PR 5114 at commit [`645a23e`](https://github.com/apache/spark/commit/645a23e380ace673dbe27a325a39b7ef1c339c4b).
     * This patch **does not merge 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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#discussion_r29175775
  
    --- Diff: streaming/src/main/scala/org/apache/spark/streaming/receiver/Receiver.scala ---
    @@ -107,9 +107,6 @@ abstract class Receiver[T](val storageLevel: StorageLevel) extends Serializable
        */
       def onStop()
     
    -  /** Override this to specify a preferred location (hostname). */
    -  def preferredLocation : Option[String] = None
    --- End diff --
    
    So with this change, custom receiver implementations will need to re-implement this code? I am not sure that is a good idea, since this is part of a stable interface. For one, that breaks custom code and the other aspect is that it sets a bad precedent.


---
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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#issuecomment-84809555
  
      [Test build #28975 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28975/consoleFull) for   PR 5114 at commit [`645a23e`](https://github.com/apache/spark/commit/645a23e380ace673dbe27a325a39b7ef1c339c4b).
     * This patch **passes all 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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#discussion_r26913307
  
    --- Diff: streaming/src/main/scala/org/apache/spark/streaming/receiver/Receiver.scala ---
    @@ -107,9 +107,6 @@ abstract class Receiver[T](val storageLevel: StorageLevel) extends Serializable
        */
       def onStop()
     
    -  /** Override this to specify a preferred location (hostname). */
    -  def preferredLocation : Option[String] = None
    --- End diff --
    
    @jerryshao thanks for your review, I have not thinked about people who customized Receiver and override this function, I just want to provide a function to set preferred location for the ReceiverInputDStream/Receiver  already exsits in Spark Streaming.


---
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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#issuecomment-84778387
  
      [Test build #28975 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28975/consoleFull) for   PR 5114 at commit [`645a23e`](https://github.com/apache/spark/commit/645a23e380ace673dbe27a325a39b7ef1c339c4b).
     * 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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#discussion_r26997367
  
    --- Diff: external/flume/src/main/scala/org/apache/spark/streaming/flume/FlumeInputDStream.scala ---
    @@ -44,12 +44,14 @@ import org.jboss.netty.handler.codec.compression._
     
     private[streaming]
     class FlumeInputDStream[T: ClassTag](
    -  @transient ssc_ : StreamingContext,
    -  host: String,
    -  port: Int,
    -  storageLevel: StorageLevel,
    -  enableDecompression: Boolean
    -) extends ReceiverInputDStream[SparkFlumeEvent](ssc_) {
    +    @transient ssc_ : StreamingContext,
    --- End diff --
    
    just for codestyle


---
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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#discussion_r26910176
  
    --- Diff: streaming/src/main/scala/org/apache/spark/streaming/receiver/Receiver.scala ---
    @@ -107,9 +107,6 @@ abstract class Receiver[T](val storageLevel: StorageLevel) extends Serializable
        */
       def onStop()
     
    -  /** Override this to specify a preferred location (hostname). */
    -  def preferredLocation : Option[String] = None
    --- End diff --
    
    Why move this function from `Receiver` to `ReceiverInputDStream`? I think for people who customized `Receiver` and override this function may fail to compile. Is there any specific reason?


---
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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#discussion_r26914212
  
    --- Diff: streaming/src/main/scala/org/apache/spark/streaming/receiver/Receiver.scala ---
    @@ -107,9 +107,6 @@ abstract class Receiver[T](val storageLevel: StorageLevel) extends Serializable
        */
       def onStop()
     
    -  /** Override this to specify a preferred location (hostname). */
    -  def preferredLocation : Option[String] = None
    --- End diff --
    
    Since `Receiver` is an interface for user, I think it would be better not remove this method, one possible way you could is to pass the argument through `ReceiverInputDStream` to this buy adding some methods.


---
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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#discussion_r26910223
  
    --- Diff: streaming/src/main/scala/org/apache/spark/streaming/dstream/ReceiverInputDStream.scala ---
    @@ -54,6 +55,14 @@ abstract class ReceiverInputDStream[T: ClassTag](@transient ssc_ : StreamingCont
     
       def stop() {}
     
    +  def setPreferredLocation(_preferredLocation: String){
    --- End diff --
    
    Space before `{`


---
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-5961][Streaming]Allow specific nodes in...

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

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


---
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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#issuecomment-84288908
  
      [Test build #28941 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28941/consoleFull) for   PR 5114 at commit [`f189b08`](https://github.com/apache/spark/commit/f189b08514c73bdc7de802324affd913b3ef474f).
     * This patch **passes all 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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#discussion_r26910218
  
    --- Diff: streaming/src/main/scala/org/apache/spark/streaming/dstream/ReceiverInputDStream.scala ---
    @@ -54,6 +55,14 @@ abstract class ReceiverInputDStream[T: ClassTag](@transient ssc_ : StreamingCont
     
       def stop() {}
     
    +  def setPreferredLocation(_preferredLocation: String){
    +    if (null != _preferredLocation){
    +      this._preferredLocation = Some(_preferredLocation)
    +    }
    --- End diff --
    
    This can be simplified as `this._preferredLocation = Option(_preferredLocation)`.


---
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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#discussion_r26997934
  
    --- Diff: external/flume/src/main/scala/org/apache/spark/streaming/flume/FlumeInputDStream.scala ---
    @@ -44,12 +44,14 @@ import org.jboss.netty.handler.codec.compression._
     
     private[streaming]
     class FlumeInputDStream[T: ClassTag](
    -  @transient ssc_ : StreamingContext,
    -  host: String,
    -  port: Int,
    -  storageLevel: StorageLevel,
    -  enableDecompression: Boolean
    -) extends ReceiverInputDStream[SparkFlumeEvent](ssc_) {
    +    @transient ssc_ : StreamingContext,
    --- End diff --
    
    Looks like changes to 4 space indent.


---
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-5961][Streaming]Allow specific nodes in...

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

    https://github.com/apache/spark/pull/5114#discussion_r26993189
  
    --- Diff: external/flume/src/main/scala/org/apache/spark/streaming/flume/FlumeInputDStream.scala ---
    @@ -44,12 +44,14 @@ import org.jboss.netty.handler.codec.compression._
     
     private[streaming]
     class FlumeInputDStream[T: ClassTag](
    -  @transient ssc_ : StreamingContext,
    -  host: String,
    -  port: Int,
    -  storageLevel: StorageLevel,
    -  enableDecompression: Boolean
    -) extends ReceiverInputDStream[SparkFlumeEvent](ssc_) {
    +    @transient ssc_ : StreamingContext,
    --- End diff --
    
    What has changed in these lines? Why are they in the diff?


---
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-5961][Streaming]Allow specific nodes in...

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

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


---
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