HeartSaVioR commented on a change in pull request #27208: [SPARK-30481][CORE]
Integrate event log compactor into Spark History Server
URL: https://github.com/apache/spark/pull/27208#discussion_r367182910
##########
File path: core/src/main/scala/org/apache/spark/internal/config/package.scala
##########
@@ -197,8 +197,6 @@ package object config {
private[spark] val EVENT_LOG_ROLLING_MAX_FILES_TO_RETAIN =
ConfigBuilder("spark.eventLog.rolling.maxFilesToRetain")
Review comment:
> Hmm... this is kind of a history server config, right? Or even more
specifically, a FsHistoryProvider config.
> Might be better to have it grouped with other configs in History.scala,
and make it a constructor argument in EventLogFileCompactor.
Yeah... the name and place was constructed when I wasn't fully sure where to
put, so I just put near neighborhood configuration. But I think it makes sense
to move to History, and also add `history` prefix into configuration name to
indicate end users that it won't run at driver side. Making it as a part of
constructor argument would be also good.
> (In fact the same applies to EVENT_LOG_COMPACTION_SCORE_THRESHOLD. Should
that be not internal() anymore, also?)
I'm not 100 % sure who would tune the score manually. Original intention was
to make it flexible to ease testing so placed it to configuration and set it to
be "internal". If we want to expose this configuration, we should maybe also
provide the information on score and the statistics so that end users can see
and tune the threshold. WDYT?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
