keith-turner commented on code in PR #36:
URL: 
https://github.com/apache/accumulo-classloaders/pull/36#discussion_r2687967864


##########
modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/util/LocalStore.java:
##########
@@ -290,12 +292,17 @@ private Path storeResource(final Resource resource) 
throws IOException {
     }
   }
 
+  private static final int DL_BUFF_SIZE = 1024 * 1024;
+
   private void downloadFile(FileResolver source, Path tempPath, Resource 
resource) {
     // CREATE_NEW ensures the temporary file name is unique for this attempt
-    // SYNC ensures file integrity on each write, in case of system failure
-    try (var in = source.getInputStream();
-        var out = Files.newOutputStream(tempPath, CREATE_NEW, WRITE, SYNC)) {
+    // SYNC ensures file integrity on each write, in case of system failure. 
Buffering minimizes
+    // system calls te read/write data which minimizes the number of syncs.
+    try (var in = new BufferedInputStream(source.getInputStream(), 
DL_BUFF_SIZE);
+        var out = new BufferedOutputStream(Files.newOutputStream(tempPath, 
CREATE_NEW, WRITE, SYNC),
+            DL_BUFF_SIZE)) {
       in.transferTo(out);
+      out.close();

Review Comment:
   Calling `fsout.getFD().sync()` once does seem better overall, but I was not 
sure how to get it to work end to end.



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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to