From 78abc3d90375aed87c9d9c9dd163d00cd9667874 Mon Sep 17 00:00:00 2001 From: zaro0508 Date: Thu, 30 May 2013 11:07:27 -0700 Subject: [PATCH] Added a Stop() method to handle stopping executor worker threads. fix for bug 1181569 Refactored how stop was handled. Removing thread and stopping thread in the same syncronized loop was causing concurrent modification error. Needed to seperate the actions for it to work. Change-Id: I99b676f377842826f9e6a847cfeff76be91c8299 --- .../plugins/gearman/AbstractWorkerThread.java | 8 ++--- .../plugins/gearman/ComputerListenerImpl.java | 16 ++------- .../hudson/plugins/gearman/GearmanProxy.java | 33 ++++++++++++++++++- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/src/main/java/hudson/plugins/gearman/AbstractWorkerThread.java b/src/main/java/hudson/plugins/gearman/AbstractWorkerThread.java index 48a5742..56a8445 100644 --- a/src/main/java/hudson/plugins/gearman/AbstractWorkerThread.java +++ b/src/main/java/hudson/plugins/gearman/AbstractWorkerThread.java @@ -20,12 +20,9 @@ package hudson.plugins.gearman; import java.util.Date; import java.util.Set; -import java.util.HashSet; -import org.gearman.worker.GearmanFunctionFactory; import org.gearman.common.GearmanNIOJobServerConnection; -import org.gearman.worker.GearmanWorker; -//import org.gearman.worker.GearmanWorkerImpl; +import org.gearman.worker.GearmanFunctionFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -118,8 +115,7 @@ public abstract class AbstractWorkerThread implements Runnable { if (worker.isRunning()) { try { - logger.info("---- Stopping " + getName() +" (" + new Date().toString() + ")"); - worker.shutdown(); + worker.stop(); } catch (Exception e) { e.printStackTrace(); } diff --git a/src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java b/src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java index 4766186..e8e7276 100644 --- a/src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java +++ b/src/main/java/hudson/plugins/gearman/ComputerListenerImpl.java @@ -23,7 +23,6 @@ import hudson.model.Computer; import hudson.slaves.ComputerListener; import java.io.IOException; -import java.util.List; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -67,19 +66,8 @@ public class ComputerListenerImpl extends ComputerListener { return; } - // remove worker when jenkins slave is deleted or disconnected - GearmanProxy gp = GearmanProxy.getInstance(); - List workers = gp.getGewtHandles(); - synchronized(workers) { - for (AbstractWorkerThread worker : workers) { - if (worker.name.contains(c.getName())) { - logger.info("---- stopping executor worker = " - + worker.getName()); - gp.getGewtHandles().remove(worker); - worker.stop(); - } - } - } + // stop worker when jenkins slave is deleted or disconnected + GearmanProxy.getInstance().stop(c); } @Override diff --git a/src/main/java/hudson/plugins/gearman/GearmanProxy.java b/src/main/java/hudson/plugins/gearman/GearmanProxy.java index bf52307..ed45bd3 100644 --- a/src/main/java/hudson/plugins/gearman/GearmanProxy.java +++ b/src/main/java/hudson/plugins/gearman/GearmanProxy.java @@ -23,6 +23,7 @@ import hudson.model.Node; import java.util.ArrayList; import java.util.Collections; +import java.util.Iterator; import java.util.List; import jenkins.model.Jenkins; @@ -142,6 +143,9 @@ public class GearmanProxy { masterName); gwt.start(); gmwtHandles.add(gwt); + + logger.info("---- Num of executors running = " + getNumExecutors()); + } /* @@ -168,8 +172,10 @@ public class GearmanProxy { //ewt.registerJobs(); ewt.start(); gewtHandles.add(ewt); - } + + logger.info("---- Num of executors running = " + getNumExecutors()); + } /* @@ -194,6 +200,31 @@ public class GearmanProxy { logger.info("---- Num of executors running = " + getNumExecutors()); } + /* + * This method stops all threads on the gewtHandles list that + * is used to service the jenkins slave/computer + * + * + * @param Node + * The Computer to stop + * + */ + public void stop(Computer computer) { + + // find the computer in the executor workers list and stop it + synchronized(gewtHandles) { + for (Iterator it = gewtHandles.iterator(); it.hasNext(); ) { + AbstractWorkerThread t = it.next(); + if (t.name.contains(computer.getName())) { + t.stop(); + it.remove(); + } + } + } + + logger.info("---- Num of executors running = " + getNumExecutors()); + } + /* * This method returns the total number of gearman executor threads */