Github user zd-project commented on a diff in the pull request:
https://github.com/apache/storm/pull/2739#discussion_r198980565
--- Diff:
storm-server/src/main/java/org/apache/storm/localizer/LocallyCachedBlob.java ---
@@ -59,57 +59,47 @@ protected LocallyCachedBlob(String blobDescription,
String blobKey) {
this.blobKey = blobKey;
}
- protected static long downloadToTempLocation(ClientBlobStore store,
String key, long currentVersion, IAdvancedFSOps fsOps,
- Function<Long, Path>
getTempPath)
- throws KeyNotFoundException, AuthorizationException, IOException {
+ /**
+ * Helper function to download blob from blob store.
+ * @param store Blob store to fetch blobs from
+ * @param key Key to retrieve blobs
+ * @param pathSupplier A function that supplies the download
destination of a blob. It guarantees the validity
+ * of path or throws {@link IOException}
+ * @param outStreamSupplier A function that supplies the {@link
OutputStream} object
+ * @return The metadata of the download session, including blob's
version and download destination
+ * @throws KeyNotFoundException Thrown if key to retrieve blob is
invalid
+ * @throws AuthorizationException Thrown if the retrieval is not under
security authorization
+ * @throws IOException Thrown if any IO error occurs
+ */
+ protected DownloadMeta fetch(ClientBlobStore store, String key,
+ IOFunction<Long, Path> pathSupplier,
+ IOFunction<File, OutputStream>
outStreamSupplier)
+ throws KeyNotFoundException, AuthorizationException,
IOException {
+
try (InputStreamWithMeta in = store.getBlob(key)) {
long newVersion = in.getVersion();
- if (newVersion == currentVersion) {
- LOG.warn("The version did not change, but going to
download again {} {}", currentVersion, key);
+ if (newVersion == getLocalVersion()) {
--- End diff --
I agree with the caching, but could this be a premature optimization?
---
