Skip to content
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.

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

Configurable number of executors #81 #174

Closed
wants to merge 1 commit into from
Closed

Configurable number of executors #81 #174

wants to merge 1 commit into from

Conversation

maafy6
Copy link

@maafy6 maafy6 commented Mar 19, 2015

Adds numExecutors to the DockerTemplate class; any slave created with this template will have that many executors. Switched the retention strategy from OnceRetentionStrategy to CloudRetentionStrategy so that, if multiple jobs are spawned on a slave, it doesn't go down when the first job finishes.

@@ -102,6 +105,7 @@

@DataBoundConstructor
public DockerTemplate(String image, String labelString,
Integer numExecutors,
Copy link
Member

Choose a reason for hiding this comment

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

Why not integer? Please check indentation

Copy link
Author

Choose a reason for hiding this comment

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

I actually had int to start, saw there were some other recent fields that used Integer so I switched it at the last moment. Argument about not null makes sense to me, I'll change and fix indent tonight.

Copy link
Member

Choose a reason for hiding this comment

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

Please remove this from constructor and add DataBoundSetter with right default value
Link #171

@KostyaSha
Copy link
Member

I think time for @DataBoundSetters! Let's check what core version do we use first...

@thomassuckow
Copy link
Contributor

I would prefer a configurable option for once vs cloud. The once strategy is very useful to me because I want a pristine environment for each build.

@KostyaSha
Copy link
Member

Yes, strategy must be choosable...

Node.Mode mode = Node.Mode.NORMAL;

RetentionStrategy retentionStrategy = new OnceRetentionStrategy(idleTerminationMinutes());
RetentionStrategy retentionStrategy = new CloudRetentionStrategy(idleTerminationMinutes());

Choose a reason for hiding this comment

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

Please make the use of CloudRetentionStrategy optional, eg. only use it when multiple executors are configured.

@maafy6
Copy link
Author

maafy6 commented Mar 20, 2015

Sounds good. Right now, I've just made it default to use OnceRetentionStrategy if there's only a single executor, otherwise use CloudRetentionStrategy. The edge case of having one executor and wanting CloudRetention seemed relatively unlikely (and, in fact, in the one case where I do have a template with one executor, it's exactly because I want a clean environment, like @thomassuckow).

@@ -9,6 +9,7 @@

String image = "image";
String labelString;
Integer numExecutors = 1;
Copy link
Member

Choose a reason for hiding this comment

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

again, use privimite

@KostyaSha
Copy link
Member

Picking all changes to my 0.9 branch. I tried to use existed code that allows choose strategy, but it lists:
hudson.slaves.RetentionStrategy.Always hudson.slaves.RetentionStrategy.Demand and SimpleScheduledRetentionStrategy...

@KostyaSha
Copy link
Member

Jenkins.getInstance().getDescriptorList(RetentionStrategy.class)

Result

Result: [hudson.slaves.RetentionStrategy$Always$DescriptorImpl@16c18976, 
hudson.slaves.SimpleScheduledRetentionStrategy$DescriptorImpl@72057e17, 
hudson.slaves.RetentionStrategy$Demand$DescriptorImpl@48858368]

@KostyaSha KostyaSha self-assigned this Mar 28, 2015
retentionStrategy = new OnceRetentionStrategy(idleTerminationMinutes());
} else {
retentionStrategy = new CloudRetentionStrategy(idleTerminationMinutes());
}
Copy link
Member

Choose a reason for hiding this comment

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

Is it make any sense? What will happen if OnceRetentionStrategy will be used with more executors?

Copy link
Member

Choose a reason for hiding this comment

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

Sent this question to maillist hope to get some comments https://groups.google.com/d/msg/jenkinsci-dev/6nGESEvJ_S0/M-jLIrHJS6AJ

Copy link
Author

Choose a reason for hiding this comment

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

I'm not 100% sure if this answers your question, but hopefully this will make sense. I have not tested with OnceRetentionStrategy and multiple executors. From the source and docs, it looked like it very much expected to be running on a slave with one executor. As far as I could tell from the source, as soon as any task was completed, it would try and terminate the slave. What I didn't want to happen was the following scenario:

  • DockerTemplate configured with two executors and OnceRetentionStrategy
  • Jenkins queues two jobs, QuickJob and LongJob
  • Jenkins spins up a new slave with that template and assigns QuickJob to one executor and LongJob to the other on that slave.
  • QuickJob completes quickly and triggers a terminate long before LongJob completes.

I wasn't sure what would happen at this point, whether or not it would let LongJob finish up before tearing down or if it would just get killed. If it's the latter, then we certainly can't allow Once with multiple executors, if it's the former, then it'd be fine (but it doesn't seem significantly different than running as Cloud with a low idle termination time).

As for the case where we're running a ContinuableExecutable and a Cloud strategy, I don't have that in my system and am not familiar enough with the paradigm to speak to it, so it is untested. (Do such tasks assume/require that they be built on the same node?)

@KostyaSha
Copy link
Member

@maafy6 could you point on code line that enforces OnceRetentionStrategy to be used only for one executor? OnceRetentionStrategy has one significant implementation check vs Cloud, it allows to keep slave when it executes durable-task.

@magnayn
Copy link
Contributor

magnayn commented Mar 31, 2015

|'m not sure I follow why you'd ever want more than one executor per slave?

Docker is good because it's extremely cheap to spin up environments that give you isolation.

I do forsee wanting to be able to start downstream builds in the same container as the previous step. What you actually want to be able to do in that scenario is to commit the container, and pass it down as the named image for subsequent step (or steps - this might be a fan-out).

I submitted a PR to jenkins-core to try to get this functionality, but PRs die a horrible death there...

@maafy6
Copy link
Author

maafy6 commented Mar 31, 2015

@magnayn See my comment on #81 as to why I think this is useful.

@KostyaSha
Copy link
Member

@magnayn what PR? I'm testing locally code that allows to choose strategies and executor numbers. I see no problem to provide it because they work in the way they implemented.

@KostyaSha
Copy link
Member

I run docker with ALWAYS strategy and it keeps this slave always online, i can set multiple executors and the main idea that i just switching from static environment creation to in-docker. Docker can be used in different ways and it should be good to provide ability choosing configurations.

@magnayn
Copy link
Contributor

magnayn commented Mar 31, 2015

So would it be fair to say that it isn't the case that you need multiple executors in order to fulfill a requirement of your build, it's purely an optimisation - likely that because of launching a slave causes the spinning up of an entire JVM to host slave.jar ?

I.E: if launching slave.jar were zero cost, this would not be a problem?

I can't remember if we pass through the CPU limiting options to docker; we probably should.

Multiple executors make me hesitant, because it feels like breaking the entire point of running inside a container. If you're not using it to throw away the environment at the end of the build - what are you even gaining by running in docker at all?

@magnayn
Copy link
Contributor

magnayn commented Mar 31, 2015

PR for slave parameters was jenkinsci/jenkins#1448 from October 2014...

@maafy6
Copy link
Author

maafy6 commented Mar 31, 2015

So would it be fair to say that it isn't the case that you need multiple executors in order to fulfill a requirement of your build, it's purely an optimisation - likely that because of launching a slave causes the spinning up of an entire JVM to host slave.jar ?

I.E: if launching slave.jar were zero cost, this would not be a problem?

I could agree with that, but with qualifications. It's not so much that it takes too much time for the JVM to spin up and et cetera, when I kicked off a build that had a lot of sub-jobs, it hosed the network on the box (even with limits on the number of containers) and it was a 50-50 shot as to whether or not Jenkins would just crash and have to be restarted. If I did not see those issues, this would not be as big of a concern for me.

As things are though, I don't want to pretend my use case is the only one that matters. The stuff @KostyaSha is doing with adding more strategies sounds fantastic to me because it will open up more ways of using it.

I can't remember if we pass through the CPU limiting options to docker; we probably should.

We can pass through CpuShares, but not CpuSet (restrict which particular CPUs).

Multiple executors make me hesitant, because it feels like breaking the entire point of running inside a container. If you're not using it to throw away the environment at the end of the build - what are you even gaining by running in docker at all?

As to what I'm gaining, I have a ton of jobs that have very similar environments. 40 some jobs that use the same label and docker image. It doesn't matter to them that the other jobs are built on that machine, but they do need that environment, and that's what docker provides. In my case, I just need sufficient, not clean.

I am sympathetic to the concern in general. Not because I think it's a misuse of docker, but because things like your PR to Jenkins get tricky in the future. In my mind, an excellent use of that would be to be able to dynmically specify volumes for a job. This is obviously incompatible with multiple executors, but I think both are still worth having and finding a way for them to co-exist.

@magnayn
Copy link
Contributor

magnayn commented Mar 31, 2015

One thing that might be affecting you is exhaustion of the /dev/random entropy pool. I'm a bit surprised that the network gets hosed though. Might be worth digging.

Thanks for explaining - I wanted to make sure I understood your challenges! I think as long as we give enough help-text around the options (and perhaps hide under 'advanced' if it isn't already it should be fine). In fact, your usecase would make a great example for 'choose this option if...'.

You'd be surprised at the number of 'I twiddled this setting I don't understand and now everything's bust' emails I get! :-)

@KostyaSha
Copy link
Member

I lost my hard-drive, will need some time to re-create choosable strategies, but i will want to fix labels handling firstly.

@KostyaSha
Copy link
Member

@maafy6 feel free to try hpi from #209 Configuration hidden under advanced button, some help pages + form validations to provide better description for end-users.

@KostyaSha
Copy link
Member

closed with #209 Please test new features and provide feedback!

@KostyaSha KostyaSha closed this Apr 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants