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.

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

Jump to bottom

Allow slave process to use the slave memory specified in the configuration #30

Closed
wants to merge 3 commits into from
Closed

Allow slave process to use the slave memory specified in the configuration #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
11edc2b
Fix slave memory allocation
Apr 29, 2014
27da5dd
Merge branch 'master' into slave
Apr 29, 2014
0f628b8
Use slave memory setting to start the jenkins slave
May 6, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/org/jenkinsci/plugins/mesos/JenkinsScheduler.java
View file
Open in desktop
Expand Up @@ -315,7 +315,7 @@ private void createMesosTask(Offer offer, Request request) {
CommandInfo
.newBuilder()
.setValue(
String.format(SLAVE_COMMAND_FORMAT, request.request.mem, request.request.jvmArgs,
String.format(SLAVE_COMMAND_FORMAT, request.request.slaveMem, request.request.jvmArgs,
getJnlpUrl(request.request.slave.name)))
.addUris(
CommandInfo.URI.newBuilder().setValue(
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/org/jenkinsci/plugins/mesos/Mesos.java
View file
Open in desktop
Expand Up @@ -14,7 +14,6 @@
*/
package org.jenkinsci.plugins.mesos;

import hudson.model.Label;

public abstract class Mesos {
private static MesosImpl mesos;
Expand All @@ -29,17 +28,21 @@ public JenkinsSlave(String name) {

public static class SlaveRequest {
JenkinsSlave slave;

final double cpus;
final int mem;
final String label;
final String jvmArgs;
final int slaveMem;

public SlaveRequest(JenkinsSlave slave, double cpus, int mem, String label, String jvmArgs) {
public SlaveRequest(JenkinsSlave slave, double cpus, int mem, String label, String jvmArgs, int slaveMem) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets make 'slaveMem' the 4th argument (after "mem") instead of last.

this.slave = slave;
this.cpus = cpus;
this.mem = mem;
this.label = label;
this.jvmArgs = jvmArgs;
// the amoutn memory for the jenkins slave process to use
Copy link
Contributor

Choose a reason for hiding this comment

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

s/the/The/
s/amoutn/amount/
s/jenkins/Jenkins/
s/use/use./

this.slaveMem = slaveMem;
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/main/java/org/jenkinsci/plugins/mesos/MesosComputerLauncher.java
View file
Open in desktop
Expand Up @@ -71,8 +71,10 @@ public void launch(SlaveComputer _computer, TaskListener listener) throws Interr
// Create the request.
double cpus = computer.getNode().getCpus();
int mem = computer.getNode().getMem();
int slaveMem = computer.getNode().getSlaveMem();

Mesos.SlaveRequest request = new Mesos.SlaveRequest(new JenkinsSlave(name),
cpus, mem, _computer.getNode().getLabelString(), computer.getJvmArgs());
cpus, mem, _computer.getNode().getLabelString(), computer.getJvmArgs(), slaveMem);
Copy link
Contributor

Choose a reason for hiding this comment

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

make slaveMem the 4th argument after "mem".


// Launch the jenkins slave.
final Lock lock = new ReentrantLock();
Expand Down
8 changes: 8 additions & 0 deletions src/main/java/org/jenkinsci/plugins/mesos/MesosSlave.java
View file
Open in desktop
Expand Up @@ -35,6 +35,7 @@ public class MesosSlave extends Slave {
private final double cpus;
private final int mem;
private final String jvmArgs;
private final int slaveMem;

private static final Logger LOGGER = Logger.getLogger(MesosSlave.class
.getName());
Expand All @@ -56,6 +57,9 @@ public MesosSlave(String name, int numExecutors, String labelString,

this.cpus = slaveCpus + (numExecutors * executorCpus);
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of passing mem and slaveJarMem which is a bit confusing how about passing around a "Resources" object. The object could contain numExecutors, slaveCpus, slaveMem, execuorCpus and executorMem. Inside JenkinsScheduler.java the "Resources" object could be handled appropriately? You could do that in a subsequent review if you don't want to do it now.

Copy link
Author

Choose a reason for hiding this comment

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

We can talk about about this in person, but there are only 3 params that we need, and putting them into a resource object doesn't seem to improve things much.
private final double cpus;
private final int mem;
private final int slaveMem;

this.mem = slaveMem + (numExecutors * executorMem);

// jenkins slave memory size and args
Copy link
Contributor

Choose a reason for hiding this comment

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

s/jenkins/Jenkins/
s/args/arguments./

this.slaveMem = slaveMem;
this.jvmArgs = jvmArgs;

LOGGER.info("Constructing Mesos slave");
Expand All @@ -68,6 +72,10 @@ public double getCpus() {
public int getMem() {
return mem;
}

public int getSlaveMem() {
return slaveMem;
}

public String getJvmArgs() {
return jvmArgs;
Expand Down