Github user tnachen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/11272#discussion_r54506572
  
    --- Diff: 
core/src/main/scala/org/apache/spark/deploy/mesos/MesosExternalShuffleService.scala
 ---
    @@ -93,7 +113,8 @@ private[mesos] class MesosExternalShuffleService(conf: 
SparkConf, securityManage
     
       protected override def newShuffleBlockHandler(
           conf: TransportConf): ExternalShuffleBlockHandler = {
    -    new MesosExternalShuffleBlockHandler(conf)
    +    val cleanerIntervalS = 
this.conf.getTimeAsSeconds("spark.shuffle.cleanerInterval", "30s")
    --- End diff --
    
    Is it a Spark convention to add a "S" in the end to tag it as Seconds? I'm 
seeing more example in the code base to have it spelled out. I'd say we should 
be consistent and use cleanerIntervalSeconds.
    Also this is only used in the Mesos shuffle service, I think should name 
space the configuration and also be consistent with other interval setting 
names, how about "spark.mesos.shuffle.cleaner.interval"


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to