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
