Github user tdas commented on a diff in the pull request: https://github.com/apache/spark/pull/15285#discussion_r83751049 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/ui/LogPage.scala --- @@ -115,6 +117,19 @@ private[ui] class LogPage(parent: WorkerWebUI) extends WebUIPage("logPage") with UIUtils.basicSparkPage(content, logType + " log page for " + pageName) } + private val fileUncompressedLengthCacheSize = parent.worker.conf.getInt( --- End diff -- I thought about the organization of the code, I dont like this. LogPage should not be aware of the details of compressing uncompressing, when Utils is doing the heavy lifting handling gzip files in a special manner. Its spreading the gzip support of log viewing between two classes unnecessary. Rather a cleaner approach is - Utils has the cache, and the cacheloader calls private util function to get gzip file size. - LogPage calls public method called `Utils.getFileSize(file, conf)`, which transparently handle compressed and non-compressed files. In the Utils, the cache should be called `compressedLogFileLengthCache`. It is initialized when `Utils.getFileSize(file, conf)` is called for the first time, using the configurations in `conf`.
--- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org