Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-7799][SPARK-12786][Streaming]Add "streaming-akka" project #10744

Closed
wants to merge 14 commits into from
Closed

[SPARK-7799][SPARK-12786][Streaming]Add "streaming-akka" project #10744

wants to merge 14 commits into from

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Jan 13, 2016

Include the following changes:

  1. Add "streaming-akka" project and org.apache.spark.streaming.akka.AkkaUtils for creating an actorStream
  2. Remove "StreamingContext.actorStream" and "JavaStreamingContext.actorStream"
  3. Update the ActorWordCount example and add the JavaActorWordCount example
  4. Make "streaming-zeromq" depend on "streaming-akka" and update the codes accordingly

@zsxwing
Add "streaming-akka" project
a5ec644 
Include the following changes:

1. Add "streaming-akka" project and org.apache.spark.streaming.akka.AkkaUtils for creating an actorStream
2. Remove "StreamingContext.actorStream" and "JavaStreamingContext.actorStream"
3. Update the ActorWordCount example and add the JavaActorWordCount example
4. Make "streaming-zeromq" depend on "streaming-akka" and update the codes accordingly
@SparkQA
Copy link

SparkQA commented Jan 13, 2016

Test build #49340 has finished for PR 10744 at commit a5ec644.

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

@SparkQA
Copy link

SparkQA commented Jan 14, 2016

Test build #49345 has finished for PR 10744 at commit c20b51a.

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

@srowen
Copy link
Member

srowen commented Jan 14, 2016

I'm probably late to the party here, but I thought Akka was simply going away? does this need to stay around in 2.x?

@zsxwing
Copy link
Member Author

zsxwing commented Jan 14, 2016

I'm probably late to the party here, but I thought Akka was simply going away? does this need to stay around in 2.x?

Yes. This PR removed actorStream so that core and streaming won't depend on Akka. However, we still want to support actorStream in an external project like Kafka and Flume.

BrianLondon
BrianLondon reviewed Jan 15, 2016
View reviewed changes
examples/src/main/scala/org/apache/spark/examples/streaming/ActorWordCount.scala
val actorSystem = AkkaUtils.createActorSystem("test", host, port.toInt, conf = conf,
securityManager = new SecurityManager(conf))._1
val akkaConf = ConfigFactory.parseString(
s"""akka.actor.provider = "akka.remote.RemoteActorRefProvider"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this change will resolve SPARK-12786.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zsxwing Could you also tag that JIRA in this PR and resolve it when this is closed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the title

@zsxwing zsxwing changed the title [SPARK-7799][Streaming]Add "streaming-akka" project [SPARK-7799][SPARK-12786][Streaming]Add "streaming-akka" project Jan 18, 2016
@zsxwing
Copy link
Member Author

zsxwing commented Jan 18, 2016

@tdas addressed your comments and also updated docs/streaming-custom-receivers.md

@zsxwing
Copy link
Member Author

zsxwing commented Jan 18, 2016

retest this please

@SparkQA
Copy link

SparkQA commented Jan 18, 2016

Test build #49631 has finished for PR 10744 at commit 4e25229.

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

@zsxwing
Copy link
Member Author

zsxwing commented Jan 18, 2016

lint-java doesn't support to add a new project :(

@zsxwing
Copy link
Member Author

zsxwing commented Jan 19, 2016

retest this please. I just disabled the java style checker.

@zsxwing zsxwing mentioned this pull request Jan 19, 2016
Closed
tdas
tdas reviewed Jan 19, 2016
View reviewed changes
examples/src/main/java/org/apache/spark/examples/streaming/JavaActorWordCount.java
jssc,
new WordcountActorSystemCreator(),
Props.create(JavaSampleActorReceiver.class, feederActorURI),
"SampleReceiver");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent. nvm. my mistake.

@SparkQA
Copy link

SparkQA commented Jan 19, 2016

Test build #49645 has finished for PR 10744 at commit 4e25229.

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

@zsxwing
Copy link
Member Author

zsxwing commented Jan 19, 2016

retest this please

tdas
tdas reviewed Jan 19, 2016
View reviewed changes
external/akka/src/main/scala/org/apache/spark/streaming/akka/AkkaUtils.scala
jssc: JavaStreamingContext,
propsForActor: Props,
actorName: String,
actorSystemCreator: JFunction0[ActorSystem]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too many variations for the Java one. Let's just have 3.
1> props, name
2> 1 + storage level // for Advanced Spark users
3> 2 + actorSystemCreator + supervisorStrategy // for advanced Spark and Akka users

@SparkQA
Copy link

SparkQA commented Jan 19, 2016

Test build #49700 has finished for PR 10744 at commit 0c8bd04.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class SampleActorReceiver[T](urlOfPublisher: String) extends ActorReceiver

tdas
tdas reviewed Jan 19, 2016
View reviewed changes
external/zeromq/src/main/scala/org/apache/spark/streaming/zeromq/ZeroMQUtils.scala
}

/**
* Create an input stream that receives messages pushed by a zeromq publisher.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too please reduce the number of createStream versions.

@tdas
Copy link
Contributor

tdas commented Jan 19, 2016

  • Add unit tests for scala and java signatures for AkkaUtils.createStream

@zsxwing
Copy link
Member Author

zsxwing commented Jan 20, 2016

retest this please

@SparkQA
Copy link

SparkQA commented Jan 20, 2016

Test build #49742 has finished for PR 10744 at commit 9e36df1.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 20, 2016

Test build #49748 has finished for PR 10744 at commit 5e2ac50.

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

@tdas
Copy link
Contributor

tdas commented Jan 20, 2016

LGTM. Will merge when tests pass. Thank @zsxwing !

@SparkQA
Copy link

SparkQA commented Jan 20, 2016

Test build #49804 has finished for PR 10744 at commit c31be35.

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

@asfgit asfgit closed this in b7d74a6 Jan 20, 2016
@zsxwing zsxwing deleted the streaming-akka-2 branch January 20, 2016 21:59
This was referenced Jan 21, 2016
Closed
Closed
asfgit pushed a commit that referenced this pull request Jan 26, 2016
@zsxwing @tdas
[SPARK-7799][STREAMING][DOCUMENT] Add the linking and deploying instr…
cbd507d 
…uctions for streaming-akka project

Since `actorStream` is an external project, we should add the linking and deploying instructions for it.

A follow up PR of #10744

Author: Shixiong Zhu <[email protected]>

Closes #10856 from zsxwing/akka-link-instruction.
@zsxwing zsxwing mentioned this pull request Jun 10, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zsxwing @SparkQA @srowen @tdas @BrianLondon