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

    https://github.com/apache/spark/pull/718#discussion_r13565262
  
    --- Diff: 
core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
    @@ -38,56 +39,75 @@ import org.apache.spark.util.Utils
      * application's event logs are maintained in the application's own 
sub-directory. This
      * is the same structure as maintained in the event log write code path in
      * EventLoggingListener.
    - *
    - * @param baseLogDir The base directory in which event logs are found
      */
     class HistoryServer(
    -    val baseLogDir: String,
    +    conf: SparkConf,
    +    provider: ApplicationHistoryProvider,
         securityManager: SecurityManager,
    -    conf: SparkConf)
    -  extends WebUI(securityManager, HistoryServer.WEB_UI_PORT, conf) with 
Logging {
    +    port: Int)
    +  extends WebUI(securityManager, port, conf) with Logging {
    +
    +  // How many applications to retain
    +  private val retainedApplications = 
conf.getInt("spark.history.retainedApplications", 50)
     
    -  import HistoryServer._
    +  // set whether to enable or disable view acls for all applications
    +  private val uiAclsEnabled = 
conf.getBoolean("spark.history.ui.acls.enable", false)
     
    -  private val fileSystem = Utils.getHadoopFileSystem(baseLogDir)
       private val localHost = Utils.localHostName()
       private val publicHost = 
Option(System.getenv("SPARK_PUBLIC_DNS")).getOrElse(localHost)
     
    -  // A timestamp of when the disk was last accessed to check for log 
updates
    -  private var lastLogCheckTime = -1L
    +  private val appLoader = new CacheLoader[String, SparkUI] {
    +    override def load(key: String): SparkUI = {
    +      val info = provider.getAppInfo(key)
    +      if (info == null) {
    +        throw new NoSuchElementException()
    +      }
    +      info.ui.getSecurityManager.setUIAcls(uiAclsEnabled)
    +      info.ui.getSecurityManager.setViewAcls(info.sparkUser, info.viewAcls)
    +      attachSparkUI(info.ui)
    +      info.ui
    +    }
    +  }
     
    -  // Number of completed applications found in this directory
    -  private var numCompletedApplications = 0
    +  private val appCache = CacheBuilder.newBuilder()
    +    .maximumSize(retainedApplications)
    +    .removalListener(new RemovalListener[String, SparkUI] {
    +      override def onRemoval(rm: RemovalNotification[String, SparkUI]) = {
    +        detachSparkUI(rm.getValue())
    +      }
    +    })
    +    .build(appLoader)
    +
    +  private val loaderServlet = new HttpServlet {
    +    protected override def doGet(req: HttpServletRequest, res: 
HttpServletResponse): Unit = {
    +      val parts = req.getPathInfo().split("/")
    --- End diff --
    
    We should do a null check here. According to the docs, `getPathInfo` 
returns null if there is no extra path information. Although we only invoke 
this servlet through `/history/*` at the moment, it would be good to keep this 
general so we don't run into random NPEs if we decide to change the path.


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

Reply via email to