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.

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

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

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4c16bf8
separate thread configurations of driver and executor for serverThrea…
jiafuzha Jan 16, 2019
81b4bf0
Revert "separate thread configurations of driver and executor for ser…
jiafuzha Jan 25, 2019
d5ec0a0
separate thread configurations of driver and executor for serverThrea…
jiafuzha Jan 16, 2019
dd8405d
correct some style issue
jiafuzha Jan 16, 2019
ab9b965
correct style issue
jiafuzha Jan 16, 2019
5427eb8
correct style issue
jiafuzha Jan 16, 2019
ecab1cc
add check for driver's legacy identifier
jiafuzha Jan 17, 2019
7519af3
override io threads with specific values for driver and executor
jiafuzha Jan 29, 2019
64e9536
correct indent
jiafuzha Jan 30, 2019
d0b319d
Merge remote-tracking branch 'upstream/master'
jiafuzha Feb 1, 2019
1f813ec
Merge branch 'master' into thread_conf_separation
jiafuzha Feb 1, 2019
0fafe21
use role instead of boolean to differentiate driver and executor
jiafuzha Feb 13, 2019
11233d2
more accurate code to look for driver and executor thread configuration
jiafuzha Mar 13, 2019
59f9922
remove SparkContext.LEGACY_DRIVER_IDENTIFIER
jiafuzha Mar 13, 2019
bedf0fe
fix binary incompatibility issue
jiafuzha Mar 13, 2019
d73cfb5
refined code
jiafuzha Mar 13, 2019
cb4e5aa
restructure code and fix executor id missing issue
jiafuzha Mar 14, 2019
8f82381
Merge branch 'master' of https://github.com/apache/spark
jiafuzha Mar 14, 2019
e4b1023
merged from master to resolve conflict
jiafuzha Mar 14, 2019
05164b9
OMerge branch 'master' of https://github.com/apache/spark
jiafuzha Apr 30, 2019
5b9d86e
Merge branch 'master' into thread_conf_separation
jiafuzha Apr 30, 2019
cb27a2c
rewrite some code and add documentation
jiafuzha Apr 30, 2019
51f78ed
miscellaneous changes
jiafuzha May 6, 2019
6bdfe6d
miscellaneous udpdate
jiafuzha May 7, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,30 @@ object SparkTransportConf {
* @param numUsableCores if nonzero, this will restrict the server and client threads to only
* use the given number of cores, rather than all of the machine's cores.
* This restriction will only occur if these properties are not already set.
* @param role optional role, could be driver, executor, worker and master. Default is
* [[None]], means no role specific configurations.
*/
def fromSparkConf(_conf: SparkConf, module: String, numUsableCores: Int = 0): TransportConf = {
def fromSparkConf(
_conf: SparkConf,
module: String,
numUsableCores: Int = 0,
role: Option[String] = None): TransportConf = {
val conf = _conf.clone

// Specify thread configuration based on our JVM's allocation of cores (rather than necessarily
// assuming we have all the machine's cores).
// NB: Only set if serverThreads/clientThreads not already set.
// specify default thread configuration based on our JVM's allocation of cores (rather than
// necessarily assuming we have all the machine's cores).
val numThreads = NettyUtils.defaultNumThreads(numUsableCores)
conf.setIfMissing(s"spark.$module.io.serverThreads", numThreads.toString)
conf.setIfMissing(s"spark.$module.io.clientThreads", numThreads.toString)
// module threads configuration
val (modServerThreads, modClientThreads) =
jiafuzha marked this conversation as resolved.
Show resolved Hide resolved
(conf.getInt(s"spark.$module.io.serverThreads", numThreads),
conf.getInt(s"spark.$module.io.clientThreads", numThreads))
// override threads configurations with role specific values if specified
val (serverThreads, clientThreads) = role match {
case Some(r) => (conf.getInt(s"spark.$r.$module.io.serverThreads", modServerThreads),
conf.getInt(s"spark.$r.$module.io.clientThreads", modClientThreads))
case None => (modServerThreads, modClientThreads)
}
conf.set(s"spark.$module.io.serverThreads", serverThreads.toString)
conf.set(s"spark.$module.io.clientThreads", clientThreads.toString)

new TransportConf(module, new ConfigProvider {
override def get(name: String): String = conf.get(name)
Expand Down
28 changes: 24 additions & 4 deletions core/src/main/scala/org/apache/spark/rpc/netty/Dispatcher.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import scala.collection.JavaConverters._
import scala.concurrent.Promise
import scala.util.control.NonFatal

import org.apache.spark.SparkException
import org.apache.spark.{SparkConf, SparkContext, SparkException}
import org.apache.spark.internal.Logging
import org.apache.spark.internal.config.Network.RPC_NETTY_DISPATCHER_NUM_THREADS
import org.apache.spark.network.client.RpcResponseCallback
Expand Down Expand Up @@ -194,12 +194,32 @@ private[netty] class Dispatcher(nettyEnv: NettyRpcEnv, numUsableCores: Int) exte
endpoints.containsKey(name)
}

/** Thread pool used for dispatching messages. */
private val threadpool: ThreadPoolExecutor = {
def getNumOfThreads(conf: SparkConf): Int = {
jiafuzha marked this conversation as resolved.
Show resolved Hide resolved
val availableCores =
if (numUsableCores > 0) numUsableCores else Runtime.getRuntime.availableProcessors()
val numThreads = nettyEnv.conf.get(RPC_NETTY_DISPATCHER_NUM_THREADS)
// module configuration
jiafuzha marked this conversation as resolved.
Show resolved Hide resolved
val modNumThreads = nettyEnv.conf.get(RPC_NETTY_DISPATCHER_NUM_THREADS)
.getOrElse(math.max(2, availableCores))
// try to get specific threads configurations of driver and executor
// override module configurations if specified
val executorId = conf.get("spark.executor.id", "")
jiafuzha marked this conversation as resolved.
Show resolved Hide resolved
// neither driver nor executor if executor id is not set
jiafuzha marked this conversation as resolved.
Show resolved Hide resolved
val role = executorId match {
case "" => ""
case SparkContext.DRIVER_IDENTIFIER => "driver"
// any other non-empty values since executor must has "spark.executor.id" set
jiafuzha marked this conversation as resolved.
Show resolved Hide resolved
case _ => "executor"
}
if (role.isEmpty) {
modNumThreads
} else {
conf.getInt(s"spark.$role.rpc.netty.dispatcher.numThreads", modNumThreads)
}
}

/** Thread pool used for dispatching messages. */
private val threadpool: ThreadPoolExecutor = {
val numThreads = getNumOfThreads(nettyEnv.conf)
val pool = ThreadUtils.newDaemonFixedThreadPool(numThreads, "dispatcher-event-loop")
for (i <- 0 until numThreads) {
pool.execute(new MessageLoop)
Expand Down
14 changes: 12 additions & 2 deletions core/src/main/scala/org/apache/spark/rpc/netty/NettyRpcEnv.scala
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import scala.reflect.ClassTag
import scala.util.{DynamicVariable, Failure, Success, Try}
import scala.util.control.NonFatal

import org.apache.spark.{SecurityManager, SparkConf}
import org.apache.spark.{SecurityManager, SparkConf, SparkContext}
import org.apache.spark.internal.Logging
import org.apache.spark.internal.config.Network._
import org.apache.spark.network.TransportContext
Expand All @@ -47,11 +47,21 @@ private[netty] class NettyRpcEnv(
host: String,
securityManager: SecurityManager,
numUsableCores: Int) extends RpcEnv(conf) with Logging {
// try to get specific threads configurations of driver and executor
val executorId = conf.get("spark.executor.id", "")
jiafuzha marked this conversation as resolved.
Show resolved Hide resolved
// neither driver nor executor if executor id is not set
jiafuzha marked this conversation as resolved.
Show resolved Hide resolved
val role = executorId match {
case "" => None
case SparkContext.DRIVER_IDENTIFIER => Some("driver")
// any other non-empty values since executor must has "spark.executor.id" set
jiafuzha marked this conversation as resolved.
Show resolved Hide resolved
case _ => Some("executor")
}

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems not necessary to separate into another line.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine (one arg per line).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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


private val dispatcher: Dispatcher = new Dispatcher(this, numUsableCores)

Expand Down
3 changes: 3 additions & 0 deletions project/MimaExcludes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ object MimaExcludes {
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.scheduler.SparkListenerApplicationStart.copy"),
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.scheduler.SparkListenerApplicationStart.this"),
ProblemFilters.exclude[MissingTypesProblem]("org.apache.spark.scheduler.SparkListenerApplicationStart$"),

// [SPARK-26632][Core] Separate Thread Configurations of Driver and Executor
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.network.netty.SparkTransportConf.fromSparkConf"),

// [SPARK-25765][ML] Add training cost to BisectingKMeans summary
ProblemFilters.exclude[DirectMissingMethodProblem]("org.apache.spark.mllib.clustering.BisectingKMeansModel.this"),
Expand Down