[SPARK-26632][Core] Separate Thread Configurations of Driver and Executor #23560

jiafuzha · 2019-01-16T06:47:52Z

What changes were proposed in this pull request?

For the below three thread configuration items applied to both driver and executor,
spark.rpc.io.serverThreads
spark.rpc.io.clientThreads
spark.rpc.netty.dispatcher.numThreads,
we separate them to driver specifics and executor specifics.
spark.driver.rpc.io.serverThreads < - > spark.executor.rpc.io.serverThreads
spark.driver.rpc.io.clientThreads < - > spark.executor.rpc.io.clientThreads
spark.driver.rpc.netty.dispatcher.numThreads < - > spark.executor.rpc.netty.dispatcher.numThreads

Spark reads these specifics first and fall back to the common configurations.

How was this patch tested?

We ran the SimpleMap app without shuffle for benchmark purpose to test Spark's scalability in HPC with omini-path NIC which has higher bandwidth than normal ethernet NIC.

Spark's base version is 2.4.0.
Spark ran in the Standalone mode. Driver was in a standalone node.
After the separation, the performance is improved a lot in 256 nodes and 512 nodes. see below test results of SimpleMapTask before and after the enhancement. You can view the tables in the JIRA too.

ds: spark.driver.rpc.io.serverThreads
dc: spark.driver.rpc.io.clientThreads
dd: spark.driver.rpc.netty.dispatcher.numThreads
ed: spark.executor.rpc.netty.dispatcher.numThreads
time: Overall Time (s)
old time: Overall Time without Separation (s)

Before:

nodes	ds	dc	dd	ed	time
128 nodes	8	8	8	8	108
256 nodes	8	8	8	8	196
512 nodes	8	8	8	8	377

After:

nodes	ds	dc	dd	ed	time	improvement
128 nodes	15	15	10	30	107	0.9%
256 nodes	12	15	10	30	159	18.8%
512 nodes	12	15	10	30	283	24.9%

carsonwang

This looks very useful in RPC performance tuning, especially on large scale. Can you please fix a few style issue?

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala

core/src/main/scala/org/apache/spark/rpc/netty/Dispatcher.scala

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala

core/src/main/scala/org/apache/spark/rpc/netty/Dispatcher.scala

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala

core/src/main/scala/org/apache/spark/rpc/netty/Dispatcher.scala

beliefer · 2019-01-17T03:27:12Z

This pr looks very clear.

carsonwang · 2019-01-22T07:25:26Z

@zjf2012 , can you please update the PR description with the performance test result?

carsonwang · 2019-01-22T07:31:25Z

cc @cloud-fan @vanzin @jerryshao, can you please help review this? In our test, configuring RPC threads separately for driver and executor are very usefully for performance, especially for large scale.

core/src/main/scala/org/apache/spark/rpc/netty/Dispatcher.scala

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala

cloud-fan · 2019-01-22T12:41:29Z

cc @zsxwing

jerryshao · 2019-01-24T10:06:25Z

Can you please fix the conflict?

jerryshao · 2019-01-24T10:06:34Z

ok to test.

jerryshao · 2019-01-24T11:37:58Z

Maybe we should also update the docs.

SparkQA · 2019-01-24T15:12:59Z

Test build #101625 has finished for PR 23560 at commit d55d6cb.

This patch passes all tests.
This patch does not merge cleanly.
This patch adds no public classes.

…ds, clientThreads and dispatcher Threads

…verThreads, clientThreads and dispatcher Threads" This reverts commit dd54f80a09dfcb92f4959b557e738a94288bd982.

…ds, clientThreads and dispatcher Threads

SparkQA · 2019-01-25T08:05:01Z

Test build #101660 has finished for PR 23560 at commit ecab1cc.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

jiafuzha · 2019-01-28T05:27:11Z

@jerryshao , I checked configuration.md as well as other related md files under the docs folder. I cannot find a proper place to put my documentation. Actually, the spark documentation even doesn't mention IO threads-related configurations. Would you please give some suggestion?

thanks.

jerryshao · 2019-01-28T08:02:48Z

OK, I see.

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala

SparkQA · 2019-01-29T12:48:58Z

Test build #101804 has finished for PR 23560 at commit 7519af3.

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

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala

core/src/main/scala/org/apache/spark/rpc/netty/Dispatcher.scala

vanzin · 2019-03-13T23:06:02Z

retest this please

SparkQA · 2019-03-13T23:13:24Z

Test build #103464 has started for PR 23560 at commit d73cfb5.

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala

core/src/main/scala/org/apache/spark/rpc/netty/Dispatcher.scala

core/src/main/scala/org/apache/spark/rpc/netty/NettyRpcEnv.scala

vanzin · 2019-03-13T23:44:20Z

@VaibhavDesai ?!?!?

VaibhavDesai · 2019-03-13T23:46:06Z

@VaibhavDesai ?!?!?

wont happen again!

SparkQA · 2019-03-14T09:11:35Z

Test build #103489 has finished for PR 23560 at commit cb4e5aa.

This patch fails Java style tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2019-03-14T19:17:51Z

Test build #103491 has finished for PR 23560 at commit e4b1023.

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

jiafuzha · 2019-04-28T04:15:28Z

@vanzin, Is this ticket ok for you to merge? any more comments? thanks.

vanzin

Please avoid adding comments that just repeat what the code is doing.

core/src/main/scala/org/apache/spark/rpc/netty/Dispatcher.scala

core/src/main/scala/org/apache/spark/rpc/netty/NettyRpcEnv.scala

vanzin · 2019-04-29T23:11:22Z

Also it would be good to update configuration.md so people actually get to know about this.

jiafuzha · 2019-04-30T05:53:25Z

@vanzin , I tried to give some documentation about this PR. Please help review.

jiafuzha · 2019-04-30T05:53:56Z

@carsonwang , please help review.

SparkQA · 2019-04-30T07:05:02Z

Test build #105023 has finished for PR 23560 at commit cb27a2c.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

core/src/main/scala/org/apache/spark/rpc/netty/NettyRpcEnv.scala

core/src/test/scala/org/apache/spark/network/netty/SparkTransportConfSuite.scala

docs/configuration.md

SparkQA · 2019-05-06T05:05:18Z

Test build #105138 has finished for PR 23560 at commit 51f78ed.

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

jerryshao · 2019-05-06T12:16:09Z

core/src/main/scala/org/apache/spark/rpc/netty/NettyRpcEnv.scala


  private[netty] val transportConf = SparkTransportConf.fromSparkConf(
    conf.clone.set(RPC_IO_NUM_CONNECTIONS_PER_PEER, 1),
    "rpc",
-    conf.get(RPC_IO_THREADS).getOrElse(numUsableCores))
+    conf.get(RPC_IO_THREADS).getOrElse(numUsableCores),
+    role)


This seems not necessary to separate into another line.

This is fine (one arg per line).

@jerryshao , The guide says,

For method declarations, use 4 space indentation for their parameters and put each in each line when the parameters don't fit in two lines.

I'll keep this code.

core/src/main/scala/org/apache/spark/rpc/netty/NettyRpcEnv.scala

vanzin · 2019-05-06T18:07:00Z

core/src/main/scala/org/apache/spark/rpc/netty/NettyRpcEnv.scala


  private[netty] val transportConf = SparkTransportConf.fromSparkConf(
    conf.clone.set(RPC_IO_NUM_CONNECTIONS_PER_PEER, 1),
    "rpc",
-    conf.get(RPC_IO_THREADS).getOrElse(numUsableCores))
+    conf.get(RPC_IO_THREADS).getOrElse(numUsableCores),
+    role)


This is fine (one arg per line).

core/src/test/scala/org/apache/spark/network/netty/SparkTransportConfSuite.scala

docs/configuration.md

SparkQA · 2019-05-07T03:48:12Z

Test build #105180 has finished for PR 23560 at commit 6bdfe6d.

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

vanzin · 2019-05-10T03:19:58Z

retest this please

SparkQA · 2019-05-10T05:38:10Z

Test build #105299 has finished for PR 23560 at commit 6bdfe6d.

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

vanzin · 2019-05-10T17:42:17Z

Merging to master.

carsonwang reviewed Jan 16, 2019

View reviewed changes

Deegue reviewed Jan 16, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala Outdated Show resolved Hide resolved

carsonwang reviewed Jan 16, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala Outdated Show resolved Hide resolved

beliefer reviewed Jan 17, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala Outdated Show resolved Hide resolved

core/src/main/scala/org/apache/spark/rpc/netty/Dispatcher.scala Outdated Show resolved Hide resolved

gcz2022 reviewed Jan 22, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/rpc/netty/Dispatcher.scala Outdated Show resolved Hide resolved

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala Outdated Show resolved Hide resolved

jiafuzha added 7 commits January 25, 2019 10:41

separate thread configurations of driver and executor for serverThrea…

4c16bf8

…ds, clientThreads and dispatcher Threads

Revert "separate thread configurations of driver and executor for ser…

81b4bf0

…verThreads, clientThreads and dispatcher Threads" This reverts commit dd54f80a09dfcb92f4959b557e738a94288bd982.

separate thread configurations of driver and executor for serverThrea…

d5ec0a0

…ds, clientThreads and dispatcher Threads

correct some style issue

dd8405d

correct style issue

ab9b965

correct style issue

5427eb8

add check for driver's legacy identifier

ecab1cc

jiafuzha force-pushed the thread_conf_separation branch from d55d6cb to ecab1cc Compare January 25, 2019 03:37

jerryshao reviewed Jan 28, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala Outdated Show resolved Hide resolved

override io threads with specific values for driver and executor

7519af3

jerryshao reviewed Jan 30, 2019

View reviewed changes

core/src/main/scala/org/apache/spark/network/netty/SparkTransportConf.scala Outdated Show resolved Hide resolved

correct indent

64e9536

jerryshao requested changes Jan 30, 2019

View reviewed changes

vanzin reviewed Mar 13, 2019

View reviewed changes

jiafuzha added 2 commits March 14, 2019 16:48

restructure code and fix executor id missing issue

cb4e5aa

Merge branch 'master' of https://github.com/apache/spark

8f82381

merged from master to resolve conflict

e4b1023

vanzin reviewed Apr 29, 2019

View reviewed changes

jiafuzha added 3 commits April 30, 2019 09:16

OMerge branch 'master' of https://github.com/apache/spark

05164b9

Merge branch 'master' into thread_conf_separation

5b9d86e

rewrite some code and add documentation

cb27a2c

vanzin reviewed Apr 30, 2019

View reviewed changes

miscellaneous changes

51f78ed

jerryshao reviewed May 6, 2019

View reviewed changes

vanzin reviewed May 6, 2019

View reviewed changes

miscellaneous udpdate

6bdfe6d

vanzin closed this in fa5dc0a May 10, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-26632][Core] Separate Thread Configurations of Driver and Executor #23560

[SPARK-26632][Core] Separate Thread Configurations of Driver and Executor #23560

jiafuzha commented Jan 16, 2019 •

edited

carsonwang left a comment

beliefer commented Jan 17, 2019

carsonwang commented Jan 22, 2019

carsonwang commented Jan 22, 2019

cloud-fan commented Jan 22, 2019

jerryshao commented Jan 24, 2019

jerryshao commented Jan 24, 2019

jerryshao commented Jan 24, 2019

SparkQA commented Jan 24, 2019

SparkQA commented Jan 25, 2019

jiafuzha commented Jan 28, 2019

jerryshao commented Jan 28, 2019

SparkQA commented Jan 29, 2019

vanzin commented Mar 13, 2019

SparkQA commented Mar 13, 2019

vanzin commented Mar 13, 2019

VaibhavDesai commented Mar 13, 2019

SparkQA commented Mar 14, 2019

SparkQA commented Mar 14, 2019

jiafuzha commented Apr 28, 2019

vanzin left a comment

vanzin commented Apr 29, 2019

jiafuzha commented Apr 30, 2019

jiafuzha commented Apr 30, 2019

SparkQA commented Apr 30, 2019

SparkQA commented May 6, 2019

jerryshao May 6, 2019

vanzin May 6, 2019

jiafuzha May 7, 2019

vanzin May 6, 2019

SparkQA commented May 7, 2019

vanzin commented May 10, 2019

SparkQA commented May 10, 2019

vanzin commented May 10, 2019

[SPARK-26632][Core] Separate Thread Configurations of Driver and Executor #23560

[SPARK-26632][Core] Separate Thread Configurations of Driver and Executor #23560

Conversation

jiafuzha commented Jan 16, 2019 • edited

What changes were proposed in this pull request?

How was this patch tested?

carsonwang left a comment

Choose a reason for hiding this comment

beliefer commented Jan 17, 2019

carsonwang commented Jan 22, 2019

carsonwang commented Jan 22, 2019

cloud-fan commented Jan 22, 2019

jerryshao commented Jan 24, 2019

jerryshao commented Jan 24, 2019

jerryshao commented Jan 24, 2019

SparkQA commented Jan 24, 2019

SparkQA commented Jan 25, 2019

jiafuzha commented Jan 28, 2019

jerryshao commented Jan 28, 2019

SparkQA commented Jan 29, 2019

vanzin commented Mar 13, 2019

SparkQA commented Mar 13, 2019

vanzin commented Mar 13, 2019

VaibhavDesai commented Mar 13, 2019

SparkQA commented Mar 14, 2019

SparkQA commented Mar 14, 2019

jiafuzha commented Apr 28, 2019

vanzin left a comment

Choose a reason for hiding this comment

vanzin commented Apr 29, 2019

jiafuzha commented Apr 30, 2019

jiafuzha commented Apr 30, 2019

SparkQA commented Apr 30, 2019

SparkQA commented May 6, 2019

jerryshao May 6, 2019

Choose a reason for hiding this comment

vanzin May 6, 2019

Choose a reason for hiding this comment

jiafuzha May 7, 2019

Choose a reason for hiding this comment

vanzin May 6, 2019

Choose a reason for hiding this comment

SparkQA commented May 7, 2019

vanzin commented May 10, 2019

SparkQA commented May 10, 2019

vanzin commented May 10, 2019

jiafuzha commented Jan 16, 2019 •

edited