From 1b6448bad8b6909a0446f46a0c32105bec10226d Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Tue, 27 Jun 2023 09:27:51 +0200 Subject: [PATCH 1/3] `RateLimitBranchProperty` follow up Since we are now using getInQueueSince for comparison and this has only millisecond resolution, tweak the comparison to detect different queue items with the same timestamp --- src/main/java/jenkins/branch/RateLimitBranchProperty.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/jenkins/branch/RateLimitBranchProperty.java b/src/main/java/jenkins/branch/RateLimitBranchProperty.java index 01767341..2f15ee3a 100644 --- a/src/main/java/jenkins/branch/RateLimitBranchProperty.java +++ b/src/main/java/jenkins/branch/RateLimitBranchProperty.java @@ -537,7 +537,7 @@ public CauseOfBlockage canRun(Queue.Item item) { return null; } for (Queue.Item i : items) { - if (i.getInQueueSince() < item.getInQueueSince()) { + if (i.getInQueueSince() <= item.getInQueueSince() && i.getId() != item.getId()) { LOGGER.log(Level.FINE, "{0} with queue id {1} blocked by queue id {2} was first", new Object[]{ job.getFullName(), From c1552d49053e6097b976d6da3726f5e727de70e0 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Tue, 27 Jun 2023 15:10:25 +0200 Subject: [PATCH 2/3] Don't go beyond the queue item --- src/main/java/jenkins/branch/RateLimitBranchProperty.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/jenkins/branch/RateLimitBranchProperty.java b/src/main/java/jenkins/branch/RateLimitBranchProperty.java index 2f15ee3a..ae9c3306 100644 --- a/src/main/java/jenkins/branch/RateLimitBranchProperty.java +++ b/src/main/java/jenkins/branch/RateLimitBranchProperty.java @@ -537,7 +537,10 @@ public CauseOfBlockage canRun(Queue.Item item) { return null; } for (Queue.Item i : items) { - if (i.getInQueueSince() <= item.getInQueueSince() && i.getId() != item.getId()) { + if (i.getId() == item.getId()) { + // We've reached the current item, no need to go further + break; + } else if (i.getInQueueSince() <= item.getInQueueSince()) { LOGGER.log(Level.FINE, "{0} with queue id {1} blocked by queue id {2} was first", new Object[]{ job.getFullName(), From 271038849faaf525c271e42dbb8e7e78fa9c7e29 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Tue, 27 Jun 2023 16:41:25 +0200 Subject: [PATCH 3/3] Switch to the other aproach --- src/main/java/jenkins/branch/RateLimitBranchProperty.java | 5 +---- .../java/jenkins/branch/RateLimitBranchPropertyTest.java | 1 + 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/jenkins/branch/RateLimitBranchProperty.java b/src/main/java/jenkins/branch/RateLimitBranchProperty.java index ae9c3306..01767341 100644 --- a/src/main/java/jenkins/branch/RateLimitBranchProperty.java +++ b/src/main/java/jenkins/branch/RateLimitBranchProperty.java @@ -537,10 +537,7 @@ public CauseOfBlockage canRun(Queue.Item item) { return null; } for (Queue.Item i : items) { - if (i.getId() == item.getId()) { - // We've reached the current item, no need to go further - break; - } else if (i.getInQueueSince() <= item.getInQueueSince()) { + if (i.getInQueueSince() < item.getInQueueSince()) { LOGGER.log(Level.FINE, "{0} with queue id {1} blocked by queue id {2} was first", new Object[]{ job.getFullName(), diff --git a/src/test/java/jenkins/branch/RateLimitBranchPropertyTest.java b/src/test/java/jenkins/branch/RateLimitBranchPropertyTest.java index 574546a1..67c4611a 100644 --- a/src/test/java/jenkins/branch/RateLimitBranchPropertyTest.java +++ b/src/test/java/jenkins/branch/RateLimitBranchPropertyTest.java @@ -248,6 +248,7 @@ public void rateLimitsConcurrentBuilds() throws Exception { assertThat(master.isInQueue(), is(false)); assertThat(master.getQueueItem(), nullValue()); QueueTaskFuture future = master.scheduleBuild2(0); + Thread.sleep(1); QueueTaskFuture future2 = master.scheduleBuild2(0, (Cause) null, new ParametersAction( Collections.singletonList(new StringParameterValue("FOO", "MANCHU"))));