From 2f7381f7b3876035b7a640db49fad552f55862ed Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2019 18:21:10 +0200 Subject: [PATCH 1/8] ThrottleQueueTaskDispatcher.java : fix whitespace --- .../ThrottleQueueTaskDispatcher.java | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 4a226d14..0bbdcaa6 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -62,20 +62,20 @@ public CauseOfBlockage canTake(Node node, Task task) { if (Jenkins.getAuthentication() == ACL.SYSTEM) { return canTakeImpl(node, task); } - + // Throttle-concurrent-builds requires READ permissions for all projects. SecurityContext orig = SecurityContextHolder.getContext(); NotSerilizableSecurityContext auth = new NotSerilizableSecurityContext(); auth.setAuthentication(ACL.SYSTEM); SecurityContextHolder.setContext(auth); - + try { return canTakeImpl(node, task); } finally { SecurityContextHolder.setContext(orig); } } - + private CauseOfBlockage canTakeImpl(Node node, Task task) { final Jenkins jenkins = Jenkins.getActiveInstance(); ThrottleJobProperty tjp = getThrottleJobProperty(task); @@ -89,7 +89,7 @@ private CauseOfBlockage canTakeImpl(Node node, Task task) { if (!pipelineCategories.isEmpty() || (tjp!=null && tjp.getThrottleEnabled())) { CauseOfBlockage cause = canRunImpl(task, tjp, pipelineCategories); if (cause != null) { - return cause; + return cause; } if (tjp != null) { if (tjp.getThrottleOption().equals("project")) { @@ -185,60 +185,60 @@ public CauseOfBlockage canRun(Queue.Item item) { } return null; } - + @Nonnull private ThrottleMatrixProjectOptions getMatrixOptions(Task task) { ThrottleJobProperty tjp = getThrottleJobProperty(task); if (tjp == null){ - return ThrottleMatrixProjectOptions.DEFAULT; + return ThrottleMatrixProjectOptions.DEFAULT; } ThrottleMatrixProjectOptions matrixOptions = tjp.getMatrixOptions(); return matrixOptions != null ? matrixOptions : ThrottleMatrixProjectOptions.DEFAULT; } - + private boolean shouldBeThrottled(@Nonnull Task task, @CheckForNull ThrottleJobProperty tjp) { - if (tjp == null) { - return false; - } - if (!tjp.getThrottleEnabled()) { - return false; - } - - // Handle matrix options - ThrottleMatrixProjectOptions matrixOptions = tjp.getMatrixOptions(); - if (matrixOptions == null) { - matrixOptions = ThrottleMatrixProjectOptions.DEFAULT; - } - if (!matrixOptions.isThrottleMatrixConfigurations() && task instanceof MatrixConfiguration) { + if (tjp == null) { return false; - } - if (!matrixOptions.isThrottleMatrixBuilds()&& task instanceof MatrixProject) { + } + if (!tjp.getThrottleEnabled()) { return false; - } - - // Allow throttling by default - return true; + } + + // Handle matrix options + ThrottleMatrixProjectOptions matrixOptions = tjp.getMatrixOptions(); + if (matrixOptions == null) { + matrixOptions = ThrottleMatrixProjectOptions.DEFAULT; + } + if (!matrixOptions.isThrottleMatrixConfigurations() && task instanceof MatrixConfiguration) { + return false; + } + if (!matrixOptions.isThrottleMatrixBuilds()&& task instanceof MatrixProject) { + return false; + } + + // Allow throttling by default + return true; } public CauseOfBlockage canRun(Task task, ThrottleJobProperty tjp, List pipelineCategories) { if (Jenkins.getAuthentication() == ACL.SYSTEM) { return canRunImpl(task, tjp, pipelineCategories); } - + // Throttle-concurrent-builds requires READ permissions for all projects. SecurityContext orig = SecurityContextHolder.getContext(); NotSerilizableSecurityContext auth = new NotSerilizableSecurityContext(); auth.setAuthentication(ACL.SYSTEM); SecurityContextHolder.setContext(auth); - + try { return canRunImpl(task, tjp, pipelineCategories); } finally { SecurityContextHolder.setContext(orig); } } - + private CauseOfBlockage canRunImpl(Task task, ThrottleJobProperty tjp, List pipelineCategories) { final Jenkins jenkins = Jenkins.getActiveInstance(); if (!shouldBeThrottled(task, tjp) && pipelineCategories.isEmpty()) { @@ -330,7 +330,7 @@ private boolean isAnotherBuildWithSameParametersRunningOnAnyNode(Queue.Item item private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.Item item) { ThrottleJobProperty tjp = getThrottleJobProperty(item.task); if (tjp == null) { - // If the property has been ocasionally deleted by this call, + // If the property has been ocasionally deleted by this call, // it does not make sense to limit the throttling by parameter. return false; } From 49eec236f11278b6e25f2a972a1dc30f8002ce4c Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2019 18:19:37 +0200 Subject: [PATCH 2/8] ThrottleQueueTaskDispatcher.java isAnotherBuildWithSameParametersRunningOnNode() : log on which node we have a hit --- .../throttleconcurrents/ThrottleQueueTaskDispatcher.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 0bbdcaa6..848d9f12 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -355,7 +355,8 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I if (executingUnitParams.containsAll(itemParams)) { LOGGER.log(Level.FINE, "build (" + exec.getCurrentWorkUnit() + ") with identical parameters (" + - executingUnitParams + ") is already running."); + executingUnitParams + ") is already running " + + "on node '" + node.getDisplayName() + "'."); return true; } } From 4c6e43e7f688577c3e65f905bd4290c703eea22c Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2019 19:30:14 +0200 Subject: [PATCH 3/8] ThrottleQueueTaskDispatcher.java isAnotherBuildWithSameParametersRunningOnNode() : log the itemParams we compare to too --- .../throttleconcurrents/ThrottleQueueTaskDispatcher.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 848d9f12..3ef9117f 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -355,7 +355,8 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I if (executingUnitParams.containsAll(itemParams)) { LOGGER.log(Level.FINE, "build (" + exec.getCurrentWorkUnit() + ") with identical parameters (" + - executingUnitParams + ") is already running " + + executingUnitParams + ") as the ones in queued item (" + + itemParams + ") is already running " + "on node '" + node.getDisplayName() + "'."); return true; } From c4b2f2e41ec6d38c0148cb2dfa1c6ce20fadf7ec Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 28 Jun 2019 19:47:22 +0200 Subject: [PATCH 4/8] ThrottleQueueTaskDispatcher.java doFilter() : log warnings in sanity check fail (some or all actual params not hit by filter) --- .../ThrottleQueueTaskDispatcher.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 3ef9117f..292f2dd9 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -382,6 +382,16 @@ private List doFilterParams(List params, List Date: Fri, 28 Jun 2019 20:06:09 +0200 Subject: [PATCH 5/8] ThrottleQueueTaskDispatcher.java isAnotherBuildWithSameParametersRunningOnNode() : log the params arrays before and after doFilter()ing --- .../ThrottleQueueTaskDispatcher.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 292f2dd9..6827b30e 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -339,7 +339,11 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I List itemParams = getParametersFromQueueItem(item); if (paramsToCompare.size() > 0) { + LOGGER.log(Level.FINE, "filter itemParams " + itemParams + + " to only pick " + paramsToCompare); itemParams = doFilterParams(paramsToCompare, itemParams); + LOGGER.log(Level.FINE, "filtering got " + itemParams.size() + + " itemParams : " + itemParams ); } if (computer != null) { @@ -350,7 +354,13 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I if (currentExecutable != null && parentTask.getOwnerTask().getName().equals(item.task.getName())) { List executingUnitParams = getParametersFromWorkUnit(exec.getCurrentWorkUnit()); + + LOGGER.log(Level.FINE, "filter executingUnitParams " + executingUnitParams + + " to only pick " + paramsToCompare); executingUnitParams = doFilterParams(paramsToCompare, executingUnitParams); + LOGGER.log(Level.FINE, "filtering got " + executingUnitParams.size() + + " executingUnitParams : " + executingUnitParams ); + } if (executingUnitParams.containsAll(itemParams)) { LOGGER.log(Level.FINE, "build (" + exec.getCurrentWorkUnit() + From 5f1af23e860d05b84a8dd294cec0d6d63a0634e3 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Mon, 10 Feb 2020 18:23:14 +0100 Subject: [PATCH 6/8] ThrottleQueueTaskDispatcher.java : rename doFilterParams() params into paramsToCompare for readability --- .../ThrottleQueueTaskDispatcher.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index 6827b30e..a52aef3a 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -380,25 +380,25 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I /** * Filter job parameters to only include parameters used for throttling */ - private List doFilterParams(List params, List OriginalParams) { - if (params.isEmpty()) { + private List doFilterParams(List paramsToCompare, List OriginalParams) { + if (paramsToCompare.isEmpty()) { return OriginalParams; } List newParams = new ArrayList(); for (ParameterValue p : OriginalParams) { - if (params.contains(p.getName())) { + if (paramsToCompare.contains(p.getName())) { newParams.add(p); } } if (newParams.size() == 0 ) { LOGGER.log(Level.WARNING, "Error selecting params, got no hits of " + - params + " in " + OriginalParams + + paramsToCompare + " in " + OriginalParams + " : is the job configuration valid?"); - } else if (newParams.size() < params.size() ) { + } else if (newParams.size() < paramsToCompare.size() ) { LOGGER.log(Level.WARNING, "Error selecting params, not all of " + - params + " were present in " + OriginalParams + + paramsToCompare + " were present in " + OriginalParams + " : is the job configuration valid?"); } From 3cadb3aede56ecba117ae82043104dc588e6e559 Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Thu, 18 Jul 2019 08:33:50 +0200 Subject: [PATCH 7/8] ThrottleQueueTaskDispatcher.java : enhance debug logging comprehensibility and update some comments --- .../ThrottleQueueTaskDispatcher.java | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index a52aef3a..c7860464 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -340,7 +340,8 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I if (paramsToCompare.size() > 0) { LOGGER.log(Level.FINE, "filter itemParams " + itemParams + - " to only pick " + paramsToCompare); + " (from queue) to only pick up to " + paramsToCompare.size() + + ": " + paramsToCompare + " (from throttle config)"); itemParams = doFilterParams(paramsToCompare, itemParams); LOGGER.log(Level.FINE, "filtering got " + itemParams.size() + " itemParams : " + itemParams ); @@ -355,12 +356,17 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I parentTask.getOwnerTask().getName().equals(item.task.getName())) { List executingUnitParams = getParametersFromWorkUnit(exec.getCurrentWorkUnit()); - LOGGER.log(Level.FINE, "filter executingUnitParams " + executingUnitParams + - " to only pick " + paramsToCompare); + LOGGER.log(Level.FINE, "filter executingUnitParams" + + " on " + computer.getDisplayName() + "#" + exec.getNumber() + + " in build (" + exec.getCurrentWorkUnit() + ") from original " + + executingUnitParams + " to only pick up to " + + paramsToCompare.size() + ": " + paramsToCompare); executingUnitParams = doFilterParams(paramsToCompare, executingUnitParams); LOGGER.log(Level.FINE, "filtering got " + executingUnitParams.size() + - " executingUnitParams : " + executingUnitParams ); - } + " executingUnitParams : " + executingUnitParams + + " on " + computer.getDisplayName() + "#" + exec.getNumber() + + " in build (" + exec.getCurrentWorkUnit() + ")"); + // if nothing filtered away, we'll compare all params, not a subset if (executingUnitParams.containsAll(itemParams)) { LOGGER.log(Level.FINE, "build (" + exec.getCurrentWorkUnit() + @@ -379,26 +385,34 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I /** * Filter job parameters to only include parameters used for throttling + * @param paramsToCompare - a list of Strings with parameter names + * @param OriginalParams - a list of ParameterValue descendants whose name fields should match + * @return a list of ParameterValue descendants whose name fields did match, entries copied from OriginalParams */ private List doFilterParams(List paramsToCompare, List OriginalParams) { if (paramsToCompare.isEmpty()) { + LOGGER.log(Level.FINE, "doFilterParams(): paramsToCompare.isEmpty() => return OriginalParams"); return OriginalParams; } List newParams = new ArrayList(); + LOGGER.log(Level.FINE, "doFilterParams(): filtering by paramsToCompare = " + paramsToCompare); for (ParameterValue p : OriginalParams) { + LOGGER.log(Level.FINE, "doFilterParams(): checking if original " + + p.getName() + " is on our list of paramsToCompare?.." ); if (paramsToCompare.contains(p.getName())) { + LOGGER.log(Level.FINE, "doFilterParams(): we have a hit in OriginalParams: " + p); newParams.add(p); } } if (newParams.size() == 0 ) { - LOGGER.log(Level.WARNING, "Error selecting params, got no hits of " + - paramsToCompare + " in " + OriginalParams + + LOGGER.log(Level.WARNING, "doFilterParams(): Error selecting params," + + " got no hits of " + paramsToCompare + " in " + OriginalParams + " : is the job configuration valid?"); } else if (newParams.size() < paramsToCompare.size() ) { - LOGGER.log(Level.WARNING, "Error selecting params, not all of " + - paramsToCompare + " were present in " + OriginalParams + + LOGGER.log(Level.WARNING, "doFilterParams(): Error selecting params," + + " not all of " + paramsToCompare + " were present in " + OriginalParams + " : is the job configuration valid?"); } From 63af186ef62037e2a8c08ae1694a495508ca0efa Mon Sep 17 00:00:00 2001 From: Jim Klimov Date: Fri, 19 Jul 2019 15:36:28 +0200 Subject: [PATCH 8/8] ThrottleQueueTaskDispatcher.java : shift logging priorities for less spam in FINE level --- .../ThrottleQueueTaskDispatcher.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java index c7860464..f9994bda 100644 --- a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java +++ b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java @@ -339,7 +339,7 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I List itemParams = getParametersFromQueueItem(item); if (paramsToCompare.size() > 0) { - LOGGER.log(Level.FINE, "filter itemParams " + itemParams + + LOGGER.log(Level.FINER, "filter itemParams " + itemParams + " (from queue) to only pick up to " + paramsToCompare.size() + ": " + paramsToCompare + " (from throttle config)"); itemParams = doFilterParams(paramsToCompare, itemParams); @@ -356,7 +356,7 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I parentTask.getOwnerTask().getName().equals(item.task.getName())) { List executingUnitParams = getParametersFromWorkUnit(exec.getCurrentWorkUnit()); - LOGGER.log(Level.FINE, "filter executingUnitParams" + + LOGGER.log(Level.FINER, "filter executingUnitParams" + " on " + computer.getDisplayName() + "#" + exec.getNumber() + " in build (" + exec.getCurrentWorkUnit() + ") from original " + executingUnitParams + " to only pick up to " + @@ -391,27 +391,27 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I */ private List doFilterParams(List paramsToCompare, List OriginalParams) { if (paramsToCompare.isEmpty()) { - LOGGER.log(Level.FINE, "doFilterParams(): paramsToCompare.isEmpty() => return OriginalParams"); + LOGGER.log(Level.FINEST, "paramsToCompare.isEmpty() => return OriginalParams"); return OriginalParams; } List newParams = new ArrayList(); - LOGGER.log(Level.FINE, "doFilterParams(): filtering by paramsToCompare = " + paramsToCompare); + LOGGER.log(Level.FINEST, "filtering by paramsToCompare = " + paramsToCompare); for (ParameterValue p : OriginalParams) { - LOGGER.log(Level.FINE, "doFilterParams(): checking if original " + + LOGGER.log(Level.FINEST, "checking if original " + p.getName() + " is on our list of paramsToCompare?.." ); if (paramsToCompare.contains(p.getName())) { - LOGGER.log(Level.FINE, "doFilterParams(): we have a hit in OriginalParams: " + p); + LOGGER.log(Level.FINEST, "we have a hit in OriginalParams: " + p); newParams.add(p); } } if (newParams.size() == 0 ) { - LOGGER.log(Level.WARNING, "doFilterParams(): Error selecting params," + + LOGGER.log(Level.WARNING, "Error selecting params," + " got no hits of " + paramsToCompare + " in " + OriginalParams + " : is the job configuration valid?"); } else if (newParams.size() < paramsToCompare.size() ) { - LOGGER.log(Level.WARNING, "doFilterParams(): Error selecting params," + + LOGGER.log(Level.WARNING, "Error selecting params," + " not all of " + paramsToCompare + " were present in " + OriginalParams + " : is the job configuration valid?"); }