Skip to content

[JENKINS-50597] Network behavior tuning IV #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carlossg merged 19 commits into from
Jun 7, 2018
Merged

[JENKINS-50597] Network behavior tuning IV #42

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a6ade80
Extend network protections to unstash.
jglick Jun 1, 2018
9580a4c
Random test failure apparently caused by a race condition between loc…
jglick Jun 1, 2018
34b020a
Figured out how to write networkExceptionUnstashing.
jglick Jun 1, 2018
25f14fa
Investigating behavior of jclouds calls in the face of network proble…
jglick Jun 4, 2018
c74fe95
Also simulating errors in stash/artifact deletion at end of build.
jglick Jun 4, 2018
9657b2d
Reducing a bit of boilerplate.
jglick Jun 4, 2018
e96d3a5
Merge branch 'master' into network-JENKINS-50597
jglick Jun 5, 2018
42e565b
Verifying & tuning behavior of metadata calls made from HTTP threads.
jglick Jun 5, 2018
6d4f272
Update parent POM to make hpi:run work better.
jglick Jun 5, 2018
c0337fe
Actually just as easy and clear to write retry logic without the guav…
jglick Jun 5, 2018
d23b8f3
Test failure after 42e565b.
jglick Jun 5, 2018
9c5d409
Switching from java.net.HttpURLConnection to Apache HTTP Client.
jglick Jun 5, 2018
71a8bb6
Extracting separate utility class RobustHTTPClient.
jglick Jun 5, 2018
cb4f2b5
Cleaner design using instance methods.
jglick Jun 5, 2018
b45134a
Picking up https://github.com/jenkinsci/apache-httpcomponents-client-…
jglick Jun 5, 2018
ecbdd89
Avoid hiding a field with a lambda param.
jglick Jun 5, 2018
a419d41
Demonstrating effectiveness of patch to ArtifactUnarchiverStepExecution.
jglick Jun 5, 2018
9b6a16a
Dependency error.
jglick Jun 5, 2018
7e3d072
Forgotten call to RobustHTTPClient.sanitize.
jglick Jun 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 8 additions & 14 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>3.12</version>
<version>3.13</version>
<relativePath />
</parent>
<groupId>io.jenkins.plugins</groupId>
Expand All @@ -19,7 +19,7 @@
<jclouds.version>2.0.3</jclouds.version>
<jenkins.version>2.121</jenkins.version>
<java.level>8</java.level>
<workflow-api-plugin.version>2.28-rc341.90cc5dc659de</workflow-api-plugin.version> <!-- TODO https://github.com/jenkinsci/workflow-api-plugin/pull/67 -->
<workflow-api-plugin.version>2.28-rc343.e9b9e0610374</workflow-api-plugin.version> <!-- TODO https://github.com/jenkinsci/workflow-api-plugin/pull/67 -->
<useBeta>true</useBeta>
</properties>

Expand Down Expand Up @@ -69,6 +69,11 @@
<artifactId>aws-java-sdk</artifactId>
<version>1.11.329</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>apache-httpcomponents-client-4-api</artifactId>
<version>4.5.5-2.2-rc32.4a9f3bcc3908</version> <!-- TODO https://github.com/jenkinsci/apache-httpcomponents-client-4-api-plugin/pull/9 -->
</dependency>
<!--
<dependency>
<groupId>org.apache.jclouds.provider</groupId>
Expand Down Expand Up @@ -150,7 +155,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-basic-steps</artifactId>
<version>2.8-rc351.c6608322f479</version> <!-- TODO https://github.com/jenkinsci/workflow-basic-steps-plugin/pull/60 -->
<version>2.8-rc353.ae434696120f</version> <!-- TODO https://github.com/jenkinsci/workflow-basic-steps-plugin/pull/60 -->
<scope>test</scope>
</dependency>
<dependency>
Expand All @@ -159,17 +164,6 @@
<version>1.7</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.github.rholder</groupId>
<artifactId>guava-retrying</artifactId>
<version>2.0.0</version>
<exclusions>
<exclusion>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>
<dependencyManagement>
<dependencies>
Expand Down
179 changes: 37 additions & 142 deletions src/main/java/io/jenkins/plugins/artifact_manager_jclouds/JCloudsArtifactManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,24 @@

package io.jenkins.plugins.artifact_manager_jclouds;

import com.github.rholder.retry.Attempt;
import com.github.rholder.retry.AttemptTimeLimiters;
import hudson.AbortException;
import hudson.EnvVars;
import hudson.FilePath;
import hudson.Launcher;
import hudson.Util;
import hudson.model.BuildListener;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.remoting.VirtualChannel;
import hudson.slaves.WorkspaceList;
import hudson.util.DirScanner;
import hudson.util.io.ArchiverFactory;
import io.jenkins.plugins.artifact_manager_jclouds.BlobStoreProvider.HttpMethod;
import io.jenkins.plugins.httpclient.RobustHTTPClient;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.HttpURLConnection;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
Expand All @@ -39,47 +50,19 @@
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.logging.Level;
import java.util.logging.Logger;

import jenkins.MasterToSlaveFileCallable;
import jenkins.model.ArtifactManager;
import jenkins.util.VirtualFile;
import org.apache.http.client.methods.HttpGet;
import org.jclouds.blobstore.BlobStore;
import org.jclouds.blobstore.BlobStoreContext;
import org.jclouds.blobstore.domain.Blob;
import org.jclouds.blobstore.domain.StorageMetadata;
import org.jclouds.blobstore.options.CopyOptions;
import org.jclouds.blobstore.options.ListContainerOptions;
import org.jenkinsci.plugins.workflow.flow.StashManager;

import com.github.rholder.retry.RetryException;
import com.github.rholder.retry.RetryListener;
import com.github.rholder.retry.RetryerBuilder;
import com.github.rholder.retry.StopStrategies;
import com.github.rholder.retry.WaitStrategies;
import com.google.common.util.concurrent.UncheckedTimeoutException;
import hudson.AbortException;
import hudson.EnvVars;
import hudson.FilePath;
import hudson.Launcher;
import hudson.Util;
import hudson.model.BuildListener;
import hudson.model.Computer;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.remoting.VirtualChannel;
import hudson.slaves.WorkspaceList;
import hudson.util.DirScanner;
import hudson.util.io.ArchiverFactory;
import io.jenkins.plugins.artifact_manager_jclouds.BlobStoreProvider.HttpMethod;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import jenkins.MasterToSlaveFileCallable;
import jenkins.model.ArtifactManager;
import jenkins.util.JenkinsJVM;
import jenkins.util.VirtualFile;
import org.apache.commons.io.IOUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

Expand All @@ -91,6 +74,8 @@ public final class JCloudsArtifactManager extends ArtifactManager implements Sta

private static final Logger LOGGER = Logger.getLogger(JCloudsArtifactManager.class.getName());

static RobustHTTPClient client = new RobustHTTPClient();

private final BlobStoreProvider provider;

private transient String key; // e.g. myorg/myrepo/master/123
Expand Down Expand Up @@ -148,11 +133,8 @@ private static class UploadToBlobStorage extends MasterToSlaveFileCallable<Void>

private final Map<String, URL> artifactUrls; // e.g. "target/x.war", "http://..."
private final TaskListener listener;
// Bind when constructed on the master side; on the agent side, deserialize those values.
private final int stopAfterAttemptNumber = UPLOAD_STOP_AFTER_ATTEMPT_NUMBER;
private final long waitMultiplier = UPLOAD_WAIT_MULTIPLIER;
private final long waitMaximum = UPLOAD_WAIT_MAXIMUM;
private final long timeout = UPLOAD_TIMEOUT;
// Bind when constructed on the master side; on the agent side, deserialize the same configuration.
private final RobustHTTPClient client = JCloudsArtifactManager.client;

UploadToBlobStorage(Map<String, URL> artifactUrls, TaskListener listener) {
this.artifactUrls = artifactUrls;
Expand All @@ -162,9 +144,7 @@ private static class UploadToBlobStorage extends MasterToSlaveFileCallable<Void>
@Override
public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
for (Map.Entry<String, URL> entry : artifactUrls.entrySet()) {
Path local = f.toPath().resolve(entry.getKey());
URL url = entry.getValue();
uploadFile(local, url, listener, stopAfterAttemptNumber, waitMultiplier, waitMaximum, timeout);
client.uploadFile(new File(f, entry.getKey()), entry.getValue(), listener);
}
return null;
}
Expand Down Expand Up @@ -229,10 +209,7 @@ private static final class Stash extends MasterToSlaveFileCallable<Integer> {
private final boolean useDefaultExcludes;
private final String tempDir;
private final TaskListener listener;
private final int stopAfterAttemptNumber = UPLOAD_STOP_AFTER_ATTEMPT_NUMBER;
private final long waitMultiplier = UPLOAD_WAIT_MULTIPLIER;
private final long waitMaximum = UPLOAD_WAIT_MAXIMUM;
private final long timeout = UPLOAD_TIMEOUT;
private final RobustHTTPClient client = JCloudsArtifactManager.client;

Stash(URL url, String includes, String excludes, boolean useDefaultExcludes, String tempDir, TaskListener listener) throws IOException {
this.url = url;
Expand All @@ -245,7 +222,7 @@ private static final class Stash extends MasterToSlaveFileCallable<Integer> {

@Override
public Integer invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
// TODO JCLOUDS-769 streaming upload is not currently straightforward, so using a temp file pending rewrite to use multipart uploads
// TODO use streaming upload rather than a temp file; is it necessary to set the content length in advance?
// (we prefer not to upload individual files for stashes, so as to preserve symlinks & file permissions, as StashManager’s default does)
Path tempDirP = Paths.get(tempDir);
Files.createDirectories(tempDirP);
Expand All @@ -258,7 +235,7 @@ public Integer invoke(File f, VirtualChannel channel) throws IOException, Interr
throw new IOException(e);
}
if (count > 0) {
uploadFile(tmp, url, listener, stopAfterAttemptNumber, waitMultiplier, waitMaximum, timeout);
client.uploadFile(tmp.toFile(), url, listener);
}
return count;
} finally {
Expand All @@ -279,24 +256,29 @@ public void unstash(String name, FilePath workspace, Launcher launcher, EnvVars
String.format("No such saved stash ‘%s’ found at %s/%s", name, provider.getContainer(), blobPath));
}
URL url = provider.toExternalURL(blob, HttpMethod.GET);
workspace.act(new Unstash(url));
workspace.act(new Unstash(url, listener));
listener.getLogger().printf("Unstashed file(s) from %s%n", provider.toURI(provider.getContainer(), blobPath));
}

private static final class Unstash extends MasterToSlaveFileCallable<Void> {
private static final long serialVersionUID = 1L;
private final URL url;
private final TaskListener listener;
private final RobustHTTPClient client = JCloudsArtifactManager.client;

Unstash(URL url) throws IOException {
Unstash(URL url, TaskListener listener) throws IOException {
this.url = url;
this.listener = listener;
}

@Override
public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
try (InputStream is = url.openStream()) {
new FilePath(f).untarFrom(is, FilePath.TarCompression.GZIP);
// Note that this API currently offers no count of files in the tarball we could report.
}
client.connect("download", "download " + url.toString().replaceFirst("[?].+$", "?…") + " into " + f, c -> c.execute(new HttpGet(url.toString())), response -> {
Copy link
Contributor

@carlossg carlossg Jun 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

… vs ... ?
should replace passwords in http://user:password@...
wouldn't be easier to parse into Url and pick the sections needed rather than regex?

Copy link
Member Author

@jglick jglick Jun 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, forgot to use sanitize here!

try (InputStream is = response.getEntity().getContent()) {
new FilePath(f).untarFrom(is, FilePath.TarCompression.GZIP);
// Note that this API currently offers no count of files in the tarball we could report.
}
}, listener);
return null;
}
}
Expand Down Expand Up @@ -359,91 +341,4 @@ private BlobStoreContext getContext() throws IOException {
return provider.getContext();
}

private static final class HTTPAbortException extends AbortException {
final int code;
HTTPAbortException(int code, String message) {
super(message);
this.code = code;
}
}

/**
* Number of upload attempts of nonfatal errors before giving up.
*/
static int UPLOAD_STOP_AFTER_ATTEMPT_NUMBER = Integer.getInteger(JCloudsArtifactManager.class.getName() + ".UPLOAD_STOP_AFTER_ATTEMPT_NUMBER", 10);
/**
* Initial number of milliseconds between first and second upload attempts.
* Subsequent ones increase exponentially.
* Note that this is not a <em>randomized</em> exponential backoff;
* and the base of the exponent is hard-coded to 2.
*/
static long UPLOAD_WAIT_MULTIPLIER = Long.getLong(JCloudsArtifactManager.class.getName() + ".UPLOAD_WAIT_MULTIPLIER", 100);
/**
* Maximum number of seconds between upload attempts.
*/
static long UPLOAD_WAIT_MAXIMUM = Long.getLong(JCloudsArtifactManager.class.getName() + ".UPLOAD_WAIT_MAXIMUM", 300);
/**
* Number of seconds to permit a single upload attempt to take.
*/
static long UPLOAD_TIMEOUT = Long.getLong(JCloudsArtifactManager.class.getName() + ".UPLOAD_TIMEOUT", /* 15m */15 * 60);

private static final ExecutorService executors = JenkinsJVM.isJenkinsJVM() ? Computer.threadPoolForRemoting : Executors.newCachedThreadPool();

/**
* Upload a file to a URL
*/
@SuppressWarnings("Convert2Lambda") // bogus use of generics (type variable should have been on class); cannot be made into a lambda
private static void uploadFile(Path f, URL url, final TaskListener listener, int stopAfterAttemptNumber, long waitMultiplier, long waitMaximum, long timeout) throws IOException, InterruptedException {
String urlSafe = url.toString().replaceFirst("[?].+$", "?…");
try {
AtomicReference<Throwable> lastError = new AtomicReference<>();
RetryerBuilder.<Void>newBuilder().
retryIfException(x -> x instanceof IOException && (!(x instanceof HTTPAbortException) || ((HTTPAbortException) x).code >= 500) || x instanceof UncheckedTimeoutException).
withRetryListener(new RetryListener() {
@Override
public <Void> void onRetry(Attempt<Void> attempt) {
if (attempt.hasException()) {
lastError.set(attempt.getExceptionCause());
}
}
}).
withStopStrategy(StopStrategies.stopAfterAttempt(stopAfterAttemptNumber)).
withWaitStrategy(WaitStrategies.exponentialWait(waitMultiplier, waitMaximum, TimeUnit.SECONDS)).
withAttemptTimeLimiter(AttemptTimeLimiters.fixedTimeLimit(timeout, TimeUnit.SECONDS, executors)).
build().call(() -> {
Throwable t = lastError.get();
if (t != null) {
listener.getLogger().println("Retrying upload after: " + (t instanceof AbortException ? t.getMessage() : t.toString()));
}
HttpURLConnection connection = (HttpURLConnection) url.openConnection();
connection.setDoOutput(true);
connection.setRequestMethod("PUT");
connection.setFixedLengthStreamingMode(Files.size(f)); // prevent loading file in memory
try (OutputStream out = connection.getOutputStream()) {
Files.copy(f, out);
}
int responseCode = connection.getResponseCode();
if (responseCode < 200 || responseCode >= 300) {
String diag;
try (InputStream err = connection.getErrorStream()) {
diag = err != null ? IOUtils.toString(err, connection.getContentEncoding()) : null;
}
throw new HTTPAbortException(responseCode, String.format("Failed to upload %s to %s, response: %d %s, body: %s", f.toAbsolutePath(), urlSafe, responseCode, connection.getResponseMessage(), diag));
}
return null;
});
} catch (ExecutionException | RetryException x) { // *sigh*, checked exceptions
Throwable x2 = x.getCause();
if (x2 instanceof IOException) {
throw (IOException) x2;
} else if (x2 instanceof RuntimeException) {
throw (RuntimeException) x2;
} else if (x2 instanceof InterruptedException) {
throw (InterruptedException) x2;
} else { // Error?
throw new RuntimeException(x);
}
}
}

}
Loading