[FIXED JENKINS-17330] FilePath.installIfNecessaryFrom should avoid ro…

…uting download over remoting channel.
jenkinsci · Mar 27, 2013 · b67272e · b67272e
1 parent 1c64ad1
commit b67272e
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 2 deletions.
diff --git a/changelog.html b/changelog.html
@@ -59,6 +59,9 @@
     Flyweight tasks should execute on the master if there's no static
     executors available.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-7291">issue 7291</a>)
+  <li class='major bug'>
+    Download tool installations directly from the slave when possible, since this is much faster than going through the master.
+    (<a href="https://issues.jenkins-ci.org/browse/JENKINS-17330">issue 17330</a>)
   <li class=bug>
     Improved UI for implicitly locked builds.
     (<a href="https://issues.jenkins-ci.org/browse/JENKINS-10197">issue 10197</a>)

diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java
@@ -493,9 +493,10 @@ public Void invoke(File dir, VirtualChannel channel) throws IOException {
         });
     }
 
-    private void unzip(File dir, InputStream in) throws IOException {
+    private static void unzip(File dir, InputStream in) throws IOException {
         File tmpFile = File.createTempFile("tmpzip", null); // uses java.io.tmpdir
         try {
+            // XXX why does this not simply use ZipInputStream?
             IOUtils.copy(in, tmpFile);
             unzip(dir,tmpFile);
         }
@@ -504,7 +505,7 @@ private void unzip(File dir, InputStream in) throws IOException {
         }
     }
 
-    private void unzip(File dir, File zipFile) throws IOException {
+    static private void unzip(File dir, File zipFile) throws IOException {
         dir = dir.getAbsoluteFile();    // without absolutization, getParentFile below seems to fail
         ZipFile zip = new ZipFile(zipFile);
         @SuppressWarnings("unchecked")
@@ -724,6 +725,19 @@ public boolean installIfNecessaryFrom(URL archive, TaskListener listener, String
             if(listener!=null)
                 listener.getLogger().println(message);
 
+            if (isRemote()) {
+                // First try to download from the slave machine.
+                try {
+                    act(new Unpack(archive));
+                    timestamp.touch(sourceTimestamp);
+                    return true;
+                } catch (IOException x) {
+                    if (listener != null) {
+                        x.printStackTrace(listener.error("Failed to download " + archive + " from slave; will retry from master"));
+                    }
+                }
+            }
+
             // for HTTP downloads, enable automatic retry for added resilience
             InputStream in = archive.getProtocol().startsWith("http") ? ProxyConfiguration.getInputStream(archive) : con.getInputStream();
             CountingInputStream cis = new CountingInputStream(in);
@@ -743,6 +757,31 @@ public boolean installIfNecessaryFrom(URL archive, TaskListener listener, String
         }
     }
 
+    private static final class Unpack implements FileCallable<Void> {
+        private final URL archive;
+        Unpack(URL archive) {
+            this.archive = archive;
+        }
+        @Override public Void invoke(File dir, VirtualChannel channel) throws IOException, InterruptedException {
+            InputStream in = archive.openStream();
+            try {
+                CountingInputStream cis = new CountingInputStream(in);
+                try {
+                    if (archive.toExternalForm().endsWith(".zip")) {
+                        unzip(dir, cis);
+                    } else {
+                        readFromTar("input stream", dir, GZIP.extract(cis));
+                    }
+                } catch (IOException x) {
+                    throw new IOException2(String.format("Failed to unpack %s (%d bytes read)", archive, cis.getByteCount()), x);
+                }
+            } finally {
+                in.close();
+            }
+            return null;
+        }
+    }
+
     /**
      * Reads the URL on the current VM, and writes all the data to this {@link FilePath}
      * (this is different from resolving URL remotely.)