mridulm commented on a change in pull request #34158:
URL: https://github.com/apache/spark/pull/34158#discussion_r720562514
##########
File path: core/src/main/scala/org/apache/spark/storage/BlockManager.scala
##########
@@ -561,7 +562,7 @@ private[spark] class BlockManager(
private def registerWithExternalShuffleServer(): Unit = {
logInfo("Registering executor with local external shuffle service.")
val shuffleManagerMeta =
- if (Utils.isPushBasedShuffleEnabled(conf)) {
+ if (Utils.isPushBasedShuffleEnabled(conf, isDriver, false)) {
Review comment:
nit: `false` -> `checkSerializer = false`
##########
File path: core/src/main/scala/org/apache/spark/storage/DiskBlockManager.scala
##########
@@ -208,7 +208,7 @@ private[spark] class DiskBlockManager(conf: SparkConf, var
deleteFilesOnStop: Bo
* permission to create directories under application local directories.
*/
private def createLocalDirsForMergedShuffleBlocks(): Unit = {
- if (Utils.isPushBasedShuffleEnabled(conf)) {
+ if (Utils.isPushBasedShuffleEnabled(conf, isDriver, false)) {
Review comment:
nit: `false` -> `checkSerializer = false`
##########
File path: core/src/main/scala/org/apache/spark/SparkEnv.scala
##########
@@ -370,7 +344,8 @@ object SparkEnv extends Logging {
} else {
None
}, blockManagerInfo,
- mapOutputTracker.asInstanceOf[MapOutputTrackerMaster])),
+ mapOutputTracker.asInstanceOf[MapOutputTrackerMaster],
+ isDriver)),
Review comment:
nit: move `isDriver` to prev line ?
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -2627,6 +2627,37 @@ private[spark] object Utils extends Logging {
}
}
+ // Create an instance of the class with the given name, possibly
initializing it with our conf
+ def instantiateClass[T](className: String, conf: SparkConf, isDriver:
Boolean): T = {
Review comment:
@rmcyang Can you rename the method as suggested by @srowen ? It is not a
general instantiation of class; but for a class which exposes one of the
supported constructor signature.
Essentially it is specific to either `Serializer` or `ShuffleManager`.
##########
File path: core/src/main/scala/org/apache/spark/util/Utils.scala
##########
@@ -2627,6 +2629,34 @@ private[spark] object Utils extends Logging {
}
}
+ // Create an instance of the class with the given name, possibly
initializing it with our conf
+ def instantiateClass[T](className: String, conf: SparkConf, isDriver:
Boolean): T = {
+ val cls = Utils.classForName(className)
+ // Look for a constructor taking a SparkConf and a boolean isDriver, then
one taking just
+ // SparkConf, then one taking no arguments
+ try {
+ cls.getConstructor(classOf[SparkConf], java.lang.Boolean.TYPE)
+ .newInstance(conf, java.lang.Boolean.valueOf(isDriver))
+ .asInstanceOf[T]
+ } catch {
+ case _: NoSuchMethodException =>
+ try {
+
cls.getConstructor(classOf[SparkConf]).newInstance(conf).asInstanceOf[T]
+ } catch {
+ case _: NoSuchMethodException =>
+ cls.getConstructor().newInstance().asInstanceOf[T]
+ }
+ }
+ }
+
+ // Create an instance of the class named by the given SparkConf property
+ // if the property is not set, possibly initializing it with our conf
+ def instantiateClassFromConf[T](propertyName: ConfigEntry[String],
Review comment:
Can this be made private to this class ? And rename to
`instantiateSerializerFromConf` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
