Skip to content
Permalink
Browse files

[FIXED JENKINS-29568] A better fix

- 252e129 doesn't work for all cases as there are code paths
  where a pending launch can be removed from the pending list and not have spent() called.
- There was no reason for iterating the list twice anyway, as all of this takes place with the locks held
- My notifying each one as we process, if there is an Error, we will not leave any stranded. The next run
  through, if there is one, will cover those instances.

(cherry picked from commit 4f0ca16)
  • Loading branch information...
stephenc authored and olivergondza committed Jul 23, 2015
1 parent f2500bd commit db1e2d2a199f7231f85809234cc1c703d3cb67a0
Showing with 35 additions and 33 deletions.
  1. +35 −33 core/src/main/java/hudson/slaves/NodeProvisioner.java
@@ -213,48 +213,50 @@ public void run() {
// bring up.

int plannedCapacitySnapshot = 0;
List<PlannedNode> completedLaunches = new ArrayList<PlannedNode>();

for (Iterator<PlannedNode> itr = pendingLaunches.iterator(); itr.hasNext(); ) {
PlannedNode f = itr.next();
if (f.future.isDone()) {
completedLaunches.add(f);
itr.remove();
try {
Node node = f.future.get();
for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
cl.onComplete(f, node);
}

jenkins.addNode(node);
LOGGER.log(Level.INFO,
"{0} provisioning successfully completed. "
+ "We have now {1,number,integer} computer(s)",
new Object[]{f.displayName, jenkins.getComputers().length});
} catch (InterruptedException e) {
throw new AssertionError(e); // since we confirmed that the future is already done
} catch (ExecutionException e) {
LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch",
e.getCause());
for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
cl.onFailure(f, e.getCause());
}
} catch (IOException e) {
LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch",
e);
for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
cl.onFailure(f, e);
}
} catch (Error e) {
// we are not supposed to try and recover from Errors
throw e;
} catch (Throwable e) {
LOGGER.log(Level.SEVERE, "Unexpected uncaught exception encountered while "
+ "processing provisioned slave " + f.displayName, e);
} finally {
itr.remove();
f.spent();
}
} else {
plannedCapacitySnapshot += f.numExecutors;
}
}

for (PlannedNode f : completedLaunches) {
try {
Node node = f.future.get();
for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
cl.onComplete(f, node);
}

jenkins.addNode(node);
LOGGER.log(Level.INFO,
"{0} provisioning successfully completed. We have now {1,number,integer} computer"
+ "(s)",
new Object[]{f.displayName, jenkins.getComputers().length});
} catch (InterruptedException e) {
throw new AssertionError(e); // since we confirmed that the future is already done
} catch (ExecutionException e) {
LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch",
e.getCause());
for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
cl.onFailure(f, e.getCause());
}
} catch (IOException e) {
LOGGER.log(Level.WARNING, "Provisioned slave " + f.displayName + " failed to launch", e);
for (CloudProvisioningListener cl : CloudProvisioningListener.all()) {
cl.onFailure(f, e);
}
} finally {
f.spent();
}
}

float plannedCapacity = plannedCapacitySnapshot;
plannedCapacitiesEMA.update(plannedCapacity);

0 comments on commit db1e2d2

Please sign in to comment.
You can’t perform that action at this time.