[SPARK-5124][Core] A standard RPC interface and an Akka implementation #4588

zsxwing · 2015-02-13T12:24:56Z

This PR added a standard internal RPC interface for Spark and an Akka implementation. See the design document for more details.

I will split the whole work into multiple PRs to make it easier for code review. This is the first PR and avoid to touch too many files.

SparkQA · 2015-02-13T12:27:51Z

Test build #27440 has started for PR 4588 at commit 45b2317.

This patch merges cleanly.

SparkQA · 2015-02-13T13:45:00Z

Test build #27440 has finished for PR 4588 at commit 45b2317.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class OutputCommitCoordinatorEndpoint(

AmplabJenkins · 2015-02-13T13:45:04Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27440/
Test PASSed.

rxin · 2015-02-24T21:16:26Z

core/src/main/scala/org/apache/spark/deploy/worker/WorkerWatcher.scala

@@ -45,30 +42,37 @@ private[spark] class WorkerWatcher(workerUrl: String)
  private var isTesting = false


can u document what this does?

Added some docs to explain it.

SparkQA · 2015-02-26T12:13:03Z

Test build #28002 has started for PR 4588 at commit 04a106e.

This patch merges cleanly.

zsxwing · 2015-02-26T12:27:34Z

I will update this PR as per discussion in https://issues.apache.org/jira/browse/SPARK-5124

SparkQA · 2015-02-26T13:33:02Z

Test build #28002 has finished for PR 4588 at commit 04a106e.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class OutputCommitCoordinatorEndpoint(

AmplabJenkins · 2015-02-26T13:33:06Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28002/
Test PASSed.

rxin · 2015-02-26T18:53:38Z

core/src/main/scala/org/apache/spark/rpc/RpcEnv.scala

+   * Retrieve the [[RpcEndpointRef]] represented by `systemName`, `address` and `endpointName`
+   */
+  def setupEndpointRef(
+       systemName: String, address: RpcAddress, endpointName: String): RpcEndpointRef


nitpick i think the indentation is off by one here.

SparkQA · 2015-03-04T12:52:52Z

Test build #28264 has started for PR 4588 at commit fe7d1ff.

This patch merges cleanly.

SparkQA · 2015-03-04T14:11:29Z

Test build #28264 has finished for PR 4588 at commit fe7d1ff.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- class OutputCommitCoordinatorEndpoint(

AmplabJenkins · 2015-03-04T14:11:33Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28264/
Test PASSed.

rxin · 2015-03-06T01:17:38Z

core/src/main/scala/org/apache/spark/rpc/RpcEnv.scala

+ * @param address the remote address of the connection which this error happens on.
+ * @param cause the cause of the network error.
+ */
+private[spark] case class NetworkErrorEvent(address: RpcAddress, cause: Throwable)


I actually like your old design better - NetworkErrorEvent is not a message, but rather just a function that can be overridden. I think it is more clear that way on how errors are handled. Similarly for AssociatedEvent and DisassociatedEvent btw.

Do you mean you like adding the methods in the previous NetworkRpcEndpoint to RpcEndpoint?

aarondav · 2015-03-28T21:50:29Z

core/src/main/scala/org/apache/spark/deploy/worker/WorkerWatcher.scala

  private[deploy] var isShutDown = false
  private[deploy] def setTesting(testing: Boolean) = isTesting = testing
  private var isTesting = false

  // Lets us filter events only from the worker's actor system
-  private val expectedHostPort = AddressFromURIString(workerUrl).hostPort
-  private def isWorker(address: Address) = address.hostPort == expectedHostPort
+  private val expectedHostPort = new java.net.URI(workerUrl)


Perhaps RpcAddress.fromUriString(workerUrl) and use of the == method?

Updated to use RpcAddress.fromUriString

aarondav · 2015-03-28T21:53:13Z

Just a few mainly minor comments, otherwise LGTM. We can iterate on some of the API moving forward as we get more endpoints using it.

Conflicts: project/MimaExcludes.scala

SparkQA · 2015-03-30T02:13:18Z

Test build #29378 has started for PR 4588 at commit 8bd1097.

SparkQA · 2015-03-30T02:18:20Z

Test build #29379 has started for PR 4588 at commit f6f3287.

aarondav · 2015-03-30T02:32:04Z

core/src/main/scala/org/apache/spark/rpc/akka/AkkaRpcEnv.scala

+    lazy val actorRef = actorSystem.actorOf(Props(new Actor with ActorLogReceive with Logging {
+
+      assert(endpointRef != null)
+      registerEndpoint(endpoint, endpointRef)


Ah - I think there was a miscommunication on theses lines. I was not worried about require vs assert, I was wondering if we could just invoke registerEndpoint(endpoint, endpointRef) right before endpointRef.init(), on L158 , since endpointRef is only used inside this call inside the constructor.

Ah, sorry. Yes, it can be moved. Done.

…pointRefByURI

SparkQA · 2015-03-30T02:43:14Z

Test build #29380 has started for PR 4588 at commit fe3df4c.

SparkQA · 2015-03-30T03:38:53Z

Test build #29378 has finished for PR 4588 at commit 8bd1097.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-03-30T03:38:58Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29378/
Test PASSed.

SparkQA · 2015-03-30T03:44:34Z

Test build #29379 has finished for PR 4588 at commit f6f3287.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-03-30T03:44:38Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29379/
Test PASSed.

SparkQA · 2015-03-30T04:09:14Z

Test build #29380 has finished for PR 4588 at commit fe3df4c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.
This patch does not change any dependencies.

AmplabJenkins · 2015-03-30T04:09:19Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/29380/
Test PASSed.

rxin · 2015-03-30T04:24:50Z

Thanks. I've merged this in master. Let's start replacing the direct use of actors with this API. Great work!

zsxwing · 2015-03-30T05:12:56Z

@rxin @CodingCat @vanzin @aarondav Thanks for reviewing this PR.

…t does not require a reply Hotfix for #4588 cc rxin Author: zsxwing <zsxwing@gmail.com> Closes #5283 from zsxwing/hotfix and squashes the following commits: cf3e5a7 [zsxwing] Move StopCoordinator to the receive method since it does not require a reply

Fixed my mistake in apache#4588 Author: zsxwing <zsxwing@gmail.com> Closes apache#5529 from zsxwing/SPARK-6934 and squashes the following commits: 9890b2d [zsxwing] Use 'spark.akka.askTimeout' for the ask timeout

A standard RPC interface and An Akka implementation

45b2317

zsxwing mentioned this pull request Feb 13, 2015

[SPARK-5124][Core][WIP] A standard internal RPC interface and an Akka implementation #3974

Closed

rxin reviewed Feb 24, 2015
View reviewed changes

zsxwing added 3 commits February 26, 2015 19:50

Change newURI to uriOf and add some comments

155b987

Remove RpcEnv.systemName

2a579f4

Remove NopCancellable and add a const NOP in object SettableCancellable

04a106e

rxin reviewed Feb 26, 2015
View reviewed changes

zsxwing added 2 commits February 27, 2015 20:07

Fix the indentation

7b9e0c9

Add explicit reply in rpc

fe7d1ff

rxin reviewed Mar 6, 2015
View reviewed changes

aarondav reviewed Mar 28, 2015
View reviewed changes

zsxwing added 4 commits March 30, 2015 09:09

Merge branch 'master' into rpc-part1

15cfd7b

Conflicts: project/MimaExcludes.scala

Move send methods above ask methods

b221398

Add RpcAddress.fromURI and rename urls to uris

f459380

Fix docs and the code style

8bd1097

Remove RpcEndpointRef.toURI

f6f3287

aarondav reviewed Mar 30, 2015
View reviewed changes

Move registerEndpoint and use actorSystem.dispatcher in asyncSetupEnd…

fe3df4c

…pointRefByURI

asfgit closed this in a8d53af Mar 30, 2015

zsxwing deleted the rpc-part1 branch March 30, 2015 05:12

zsxwing mentioned this pull request Mar 31, 2015

[SPARK-5124][Core] Move StopCoordinator to the receive method since it does not require a reply #5283

Closed

zsxwing mentioned this pull request Apr 15, 2015

[SPARK-6934][Core] Use 'spark.akka.askTimeout' for the ask timeout #5529

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-5124][Core] A standard RPC interface and an Akka implementation #4588

[SPARK-5124][Core] A standard RPC interface and an Akka implementation #4588

zsxwing commented Feb 13, 2015

SparkQA commented Feb 13, 2015

SparkQA commented Feb 13, 2015

AmplabJenkins commented Feb 13, 2015

rxin Feb 24, 2015

zsxwing Feb 26, 2015

SparkQA commented Feb 26, 2015

zsxwing commented Feb 26, 2015

SparkQA commented Feb 26, 2015

AmplabJenkins commented Feb 26, 2015

rxin Feb 26, 2015

SparkQA commented Mar 4, 2015

SparkQA commented Mar 4, 2015

AmplabJenkins commented Mar 4, 2015

rxin Mar 6, 2015

zsxwing Mar 6, 2015

rxin Mar 6, 2015

aarondav Mar 28, 2015

zsxwing Mar 30, 2015

aarondav commented Mar 28, 2015

SparkQA commented Mar 30, 2015

SparkQA commented Mar 30, 2015

aarondav Mar 30, 2015

zsxwing Mar 30, 2015

SparkQA commented Mar 30, 2015

SparkQA commented Mar 30, 2015

AmplabJenkins commented Mar 30, 2015

SparkQA commented Mar 30, 2015

AmplabJenkins commented Mar 30, 2015

SparkQA commented Mar 30, 2015

AmplabJenkins commented Mar 30, 2015

rxin commented Mar 30, 2015

zsxwing commented Mar 30, 2015

		@@ -45,30 +42,37 @@ private[spark] class WorkerWatcher(workerUrl: String)
		private var isTesting = false

[SPARK-5124][Core] A standard RPC interface and an Akka implementation #4588

[SPARK-5124][Core] A standard RPC interface and an Akka implementation #4588

Conversation

zsxwing commented Feb 13, 2015

SparkQA commented Feb 13, 2015

SparkQA commented Feb 13, 2015

AmplabJenkins commented Feb 13, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Feb 26, 2015

zsxwing commented Feb 26, 2015

SparkQA commented Feb 26, 2015

AmplabJenkins commented Feb 26, 2015

Choose a reason for hiding this comment

SparkQA commented Mar 4, 2015

SparkQA commented Mar 4, 2015

AmplabJenkins commented Mar 4, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aarondav commented Mar 28, 2015

SparkQA commented Mar 30, 2015

SparkQA commented Mar 30, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Mar 30, 2015

SparkQA commented Mar 30, 2015

AmplabJenkins commented Mar 30, 2015

SparkQA commented Mar 30, 2015

AmplabJenkins commented Mar 30, 2015

SparkQA commented Mar 30, 2015

AmplabJenkins commented Mar 30, 2015

rxin commented Mar 30, 2015

zsxwing commented Mar 30, 2015