JENKINS-50429 combine env vars into a single set before writing once

[mrjgreen]

• combine env vars before writing once into the container

[mrjgreen]

@carlossg - I tested this locally and it appears to have cut the time down to 1 second overhead instead of 3/4 seconds. As far as I can tell, the env vars are still being set... Does this look reasonable?

[carlossg]

makes sense. But I think I have tested something similar and didn't improve the speed, need to retest

[mrjgreen]

👍 - I've only tested this on a very simple test case. We are attempting to test it against a large pipeline soon.

Is there anything we can do to address the root cause of the problem with the input.write() buffer? Would upgrading the JDK version in the jenkins containers be likely to help?

I'll look into this and see if the OutputStream known issue was ever resolved in any version.

I've had a quick look into this and it doesn't seem like this was ever resolved:

https://github.com/dmlloyd/openjdk/blob/jdk/jdk10/src/java.base/share/classes/java/io/PipedInputStream.java#L273

[sylus]

Was wondering if there was any other progress against this?

[mrjgreen]

Our tests suggest that this does reduce the overhead of running sh commands - it reduced the overhead of a date sh command from 4 seconds to 2 seconds.

This is still not ideal. I think the problem needs raising with the fabricate K8s client to see if we can use an alternative to the Input Stream (which is where the problem originates).

https://github.com/fabric8io/kubernetes-client/blob/59e5b9d0f318f4f78f417efaec34050ed8122b8f/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/ExecWatch.java#L26

[jeff-knurek]

also wondering if there was any other progress against this....

[carlossg]

Better than asking for progress it'd be good if you tried the pr and comment on whether it fixes the problem for you or not

[carlossg]

I have tested with this job and I'm still getting a big difference between shell execution inside a container.

60 calls outside container	60 calls in container	1 call in container
19s	53s	1s

def label = "exec-${UUID.randomUUID().toString()}"
podTemplate(label: label) {

    timestamps {
        node(label){
            stage('60 calls outside container'){
                for (i = 0; i <60; i++) {
                    sh "echo $i"
                }
            }
            stage('60 calls in container'){
                container("jnlp"){
                    for (i = 0; i <60; i++) {
                        sh "echo $i"
                    }
                }
            }
            stage('1 call in container'){
                container("jnlp"){
                    sh "for i in `seq 1 100`; do echo $i; done"
                }
            }
        }
    }
}

[carlossg]

Merging because it doesn't hurt