Skip to content

Commit

Permalink
[JENKINS-30269] Add inversePrecedence parameter + tests
Browse files Browse the repository at this point in the history
  • Loading branch information
amuniz committed Mar 23, 2016
1 parent a14014b commit 9fe73b1
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 90 deletions.
Expand Up @@ -15,7 +15,7 @@ public class LockStep extends AbstractStepImpl implements Serializable {

public final String resource;

public Integer maxWaiting;
public boolean inversePrecedence = false;

@DataBoundConstructor
public LockStep(String resource) {
Expand All @@ -29,8 +29,8 @@ public LockStep(String resource) {
}

@DataBoundSetter
public void setMaxWaiting(Integer maxWaiting) {
this.maxWaiting = maxWaiting;
public void setInversePrecedence(boolean inversePrecedence) {
this.inversePrecedence = inversePrecedence;
}

@Extension
Expand All @@ -47,7 +47,7 @@ public String getFunctionName() {

@Override
public String getDisplayName() {
return "Lock shared resources to manage concurrency";
return "Lock shared resource";
}

@Override
Expand Down
Expand Up @@ -5,7 +5,6 @@

import org.jenkins.plugins.lockableresources.queue.LockableResourcesStruct;
import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
import org.jenkinsci.plugins.workflow.steps.BodyExecution;
import org.jenkinsci.plugins.workflow.steps.BodyExecutionCallback;
import org.jenkinsci.plugins.workflow.steps.StepContext;
import org.jenkinsci.plugins.workflow.steps.StepContextParameter;
Expand All @@ -26,23 +25,20 @@ public class LockStepExecution extends AbstractStepExecutionImpl {
@StepContextParameter
private transient TaskListener listener;

private volatile BodyExecution body;
private static final Logger LOGGER = Logger.getLogger(LockStepExecution.class.getName());

@Override
public boolean start() throws Exception {
listener.getLogger().println("Trying to acquire lock on [" + step.resource + "]");
LockableResourcesStruct resourceHolder = new LockableResourcesStruct(step.resource);
if(LockableResourcesManager.get().lock(resourceHolder.required, run, getContext())) {
proceed(getContext(), step.resource);
} else {
if(!LockableResourcesManager.get().lock(resourceHolder.required, run, getContext(), step.inversePrecedence)) {
// we have to wait
listener.getLogger().println("Resource locked, waiting...");
}
listener.getLogger().println("[" + step.resource + "] is locked, waiting...");
} // proceed is called inside lock otherwise
return false;
}

public static void proceed(StepContext context, String resource) {
public static void proceed(StepContext context, String resource, boolean inversePrecedence) {
LockableResourcesStruct resourceHolder = new LockableResourcesStruct(resource);
Run<?, ?> r = null;
try {
Expand All @@ -55,7 +51,7 @@ public static void proceed(StepContext context, String resource) {

LOGGER.finest("Lock acquired on [" + resource + "] by " + r.getExternalizableId());
context.newBodyInvoker().
withCallback(new Callback(resourceHolder, r)).
withCallback(new Callback(resourceHolder, r, inversePrecedence)).
withDisplayName(null).
start();
}
Expand All @@ -65,12 +61,14 @@ private static final class Callback extends BodyExecutionCallback.TailCall {
private final LockableResourcesStruct resourceHolder;
private transient Run<?, ?> run;
private final String buildExternalizableId;
private final boolean inversePrecedence;

Callback(LockableResourcesStruct resourceHolder, Run<?, ?> run) {
Callback(LockableResourcesStruct resourceHolder, Run<?, ?> run, boolean inversePrecedence) {
// It's granted to contain one item (and only one for now)
this.resourceHolder = resourceHolder;
this.run = run;
this.buildExternalizableId = run.getExternalizableId();
this.inversePrecedence = inversePrecedence;
}

protected void finished(StepContext context) throws Exception {
Expand All @@ -81,10 +79,9 @@ private void unlock(StepContext context) throws IOException, InterruptedExceptio
if (run == null && buildExternalizableId != null) {
run = Run.fromExternalizableId(buildExternalizableId);
}
LockableResourcesManager.get().unlock(resourceHolder.required, run, context);
LockableResourcesManager.get().unlock(resourceHolder.required, run, context, inversePrecedence);
context.get(TaskListener.class).getLogger().println("Lock released on resouce [" + resourceHolder.required.get(0) + "]");
LOGGER.finest("Lock released on [" + resourceHolder.required.get(0) + "]");
context.onSuccess(null);
}

private static final long serialVersionUID = 1L;
Expand Down
Expand Up @@ -111,7 +111,7 @@ public String getLabels() {
}


public Integer getBuildsInQueue() {
public Integer getContextsInQueue() {
return queuedContexts.size();
}

Expand Down Expand Up @@ -298,46 +298,32 @@ public void queueAdd(StepContext context) {
* if there are no more contexts for that build.
*/
@CheckForNull
public StepContext getNextQueuedContext() {
public StepContext getNextQueuedContext(boolean inversePrecedence) {
if (queuedContexts.size() > 0) {
StepContext nextContext = queuedContexts.remove(0);
Run<?, ?> nextContextRun;
try {
nextContextRun = nextContext.get(Run.class);
} catch (Exception e) {
// This should not happen, but let's remove it and get the next
queuedContexts.remove(nextContext);
return getNextQueuedContext();
if (!inversePrecedence) {
return queuedContexts.remove(0);
} else {
int newest = 0;
int index = 0;
int newestIndex = 0;
for (StepContext c : queuedContexts) {
try {
Run<?, ?> run = c.get(Run.class);
if (run.getNumber() > newest) {
newest = run.getNumber();
newestIndex = index;
}
} catch (Exception e) {
// skip this one
}
index++;
}
return queuedContexts.remove(newestIndex);
}
if (!isThereAnotherContextForTheSameRun(nextContextRun)) {
setBuild(null);
}
return nextContext;
}
return null;
}

private boolean isThereAnotherContextForTheSameRun(Run<?, ?> nextContextRun) {
boolean thereIsAnotherContextForTheSameRun = false;
Iterator<StepContext> it = queuedContexts.iterator();
while (it.hasNext()) {
StepContext c = it.next();
Run<?, ?> waitingRun;
try {
waitingRun = c.get(Run.class);
} catch (Exception e) {
// this should not happen, but let's remove it anyway
it.remove();
continue;
}
if (nextContextRun.getExternalizableId().equals(waitingRun.getExternalizableId())) {
thereIsAnotherContextForTheSameRun = true;
break;
}
}
return thereIsAnotherContextForTheSameRun;
}

@DataBoundSetter
public void setReservedBy(String userName) {
this.reservedBy = Util.fixEmptyAndTrim(userName);
Expand Down
Expand Up @@ -15,6 +15,7 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
Expand Down Expand Up @@ -208,12 +209,16 @@ private boolean checkCurrentResourcesStatus(List<LockableResource> selected,
}
return true;
}

public synchronized boolean lock(List<LockableResource> resources, Run<?, ?> build, @Nullable StepContext context) {
return lock(resources, build, context, false);
}

/**
* Try to lock the resource and return true if locked.
*/
public synchronized boolean lock(List<LockableResource> resources,
Run<?, ?> build, @Nullable StepContext context) {
Run<?, ?> build, @Nullable StepContext context, boolean inversePrecedence) {
boolean needToWait = false;
for (LockableResource r : resources) {
if (r.isReserved() || r.isLocked()) {
Expand All @@ -229,17 +234,22 @@ public synchronized boolean lock(List<LockableResource> resources,
r.unqueue();
r.setBuild(build);
if (context != null) {
LockStepExecution.proceed(context, r.getName());
LockStepExecution.proceed(context, r.getName(), inversePrecedence);
break;
}
}
}
save();
return !needToWait;
}

public synchronized void unlock(List<LockableResource> resourcesToUnLock,
Run<?, ?> build, @Nullable StepContext context) {
unlock(resourcesToUnLock, build, context, false);
}

public synchronized void unlock(List<LockableResource> resourcesToUnLock,
Run<?, ?> build, @Nullable StepContext context, boolean inversePrecedence) {
for (LockableResource r : resourcesToUnLock) {
// Search the resource in the internal list to unlock it
for (LockableResource internal : resources) {
Expand All @@ -248,12 +258,17 @@ public synchronized void unlock(List<LockableResource> resourcesToUnLock,
(internal.getBuild() != null && build.getExternalizableId().equals(internal.getBuild().getExternalizableId()))) {
if (context != null) {
// this will remove the context from the queue and setBuild(null) if there are no more contexts
StepContext nextContext = internal.getNextQueuedContext();
StepContext nextContext = internal.getNextQueuedContext(inversePrecedence);
if (nextContext != null) {
LockStepExecution.proceed(nextContext, internal.getName());
if (internal.getBuildsInQueue() > 0) {
internal.setBuild(null);
try {
internal.setBuild(nextContext.get(Run.class));
} catch (Exception e) {
throw new IllegalStateException("Can not access the context of a running build", e);
}
LockStepExecution.proceed(nextContext, internal.getName(), inversePrecedence);
} else {
// No more contexts, unlock resource
internal.setBuild(null);
}
break;
} else {
Expand Down

0 comments on commit 9fe73b1

Please sign in to comment.