Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-7813]
Fixed the throughput problem between master/slave communication.
This fix contains two independent problems.

One was in the remoting. During a large sustained data transfer
(such as artifact archiving and large test reports), the way we
were doing flow control and ACK-ing were penalizing us badly.
I improved the flow control algorithm in remoting 1.23, and also
increased advertised window size so that the transfer can saturate
available bandwidth even when a latency is large. (And unless
the reader side is excessivesly slow, this shouldn't increase
any memory consumption.)

The other fix was in trilead-ssh2, which is our SSH client
implementation used by ssh-slaves plugin. The buffer size for flow
control was too small. I improved the way buffering is done to reduce
the memory footprint when the reader closely follows the writer, then I
increased the advertised window size. Again, this shouldn't increase
memory consumption (in fact it'll likely actually reduce them) unless
the reader end gets abandoned.

On my simulated latency-injected network, the sustained transfer rate is
now on par with scp. We win for smaller files because of the TCP slow
start penality that scp would incur, and we lose a bit as files get
larger due to additional framing overhead.

If you have manually extracted slave.jar and placed them on slaves, you
need to update them to 2.23 to see the performance benefits.
  • Loading branch information
kohsuke committed Mar 21, 2013
1 parent c49d6b5 commit 8a3e909
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 3 deletions.
3 changes: 3 additions & 0 deletions changelog.html
Expand Up @@ -64,6 +64,9 @@
<li class=bug>
Master node mode not correctly displayed in <code>/computer/(master)/configure</code>.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-17263">issue 17263</a>)
<li class='major rfe'>
Performance improvement in master/slave communication throughput
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-7813">issue 7813</a>)
</ul>
</div><!--=TRUNK-END=-->

Expand Down
2 changes: 1 addition & 1 deletion core/pom.xml
Expand Up @@ -125,7 +125,7 @@ THE SOFTWARE.
<dependency>
<groupId>org.jenkins-ci</groupId>
<artifactId>trilead-ssh2</artifactId>
<version>build214-jenkins-1</version>
<version>build214-jenkins-2</version>
</dependency>
<dependency>
<groupId>org.kohsuke.stapler</groupId>
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/hudson/tasks/ArtifactArchiver.java
Expand Up @@ -32,6 +32,7 @@
import hudson.model.BuildListener;
import hudson.model.Result;
import hudson.util.FormValidation;
import hudson.util.TimeUnit2;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.AncestorInPath;
Expand Down Expand Up @@ -122,6 +123,7 @@ public boolean perform(AbstractBuild<?,?> build, Launcher launcher, BuildListene
File dir = build.getArtifactsDir();
dir.mkdirs();

final long start = System.nanoTime();

This comment has been minimized.

Copy link
@jglick

jglick Mar 21, 2013

Member

I guess this was left in by accident, kill it (and comment below and import).

listener.getLogger().println(Messages.ArtifactArchiver_ARCHIVING_ARTIFACTS());
try {
FilePath ws = build.getWorkspace();
Expand Down Expand Up @@ -157,6 +159,8 @@ public boolean perform(AbstractBuild<?,?> build, Launcher launcher, BuildListene
return true;
}

// System.out.println("Took "+ TimeUnit2.NANOSECONDS.toMillis(System.nanoTime()-start));

return true;
}

Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -175,7 +175,7 @@ THE SOFTWARE.
<dependency>
<groupId>org.jenkins-ci.main</groupId>
<artifactId>remoting</artifactId>
<version>2.22</version>
<version>2.23</version>
</dependency>

<dependency>
Expand Down
14 changes: 13 additions & 1 deletion war/pom.xml
Expand Up @@ -271,7 +271,19 @@ THE SOFTWARE.
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>ssh-slaves</artifactId>
<!-- when upgrading may need to add credentials and ssh-credentials too: https://github.com/jenkinsci/ssh-slaves-plugin/commit/7f7031bdaae528baf8e4a1cf29a95849ef268c5c -->

This comment has been minimized.

Copy link
@jglick

jglick Mar 21, 2013

Member

Delete the comment above which is now obsolete because you have done it!

<version>0.22</version>
<version>0.23</version>
<type>hpi</type>
</artifactItem>
<artifactItem>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>credentials</artifactId>
<version>1.3.1</version>
<type>hpi</type>
</artifactItem>
<artifactItem>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>ssh-credentials</artifactId>
<version>0.2</version>
<type>hpi</type>
</artifactItem>
<artifactItem>
Expand Down

0 comments on commit 8a3e909

Please sign in to comment.