Merge "Add support for custom quota-exceeded messages"
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/MaxConnectionsLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/quota/MaxConnectionsLimiter.java
index f2b18cd..a5245ec 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/MaxConnectionsLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/MaxConnectionsLimiter.java
@@ -1,3 +1,17 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
package com.googlesource.gerrit.plugins.quota;
import com.google.gerrit.httpd.AllRequestFilter;
@@ -7,10 +21,11 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
-import org.eclipse.jgit.lib.Config;
-
import java.io.IOException;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Matcher;
@@ -21,6 +36,7 @@
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.eclipse.jgit.lib.Config;
@Singleton
public class MaxConnectionsLimiter extends AllRequestFilter {
@@ -53,9 +69,11 @@
Config cfg = projectCache.getAllProjects().getConfig("quota.config").get();
for (String group : cfg.getSubsections(AccountLimitsConfig.GROUP_SECTION)) {
String val = cfg.getString(AccountLimitsConfig.GROUP_SECTION, group, CONFIG_KEY);
- Matcher matcher = REST_API_REGEX.matcher(val);
- if (matcher.matches()) {
- limits.add(new Limit(group, Integer.parseInt(matcher.group(1))));
+ if (val != null) {
+ Matcher matcher = REST_API_REGEX.matcher(val);
+ if (matcher.matches()) {
+ limits.add(new Limit(group, Integer.parseInt(matcher.group(1))));
+ }
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/MinStartForQueueQuota.java b/src/main/java/com/googlesource/gerrit/plugins/quota/MinStartForQueueQuota.java
index 33b9a3f..68cdc1d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/MinStartForQueueQuota.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/MinStartForQueueQuota.java
@@ -14,15 +14,15 @@
package com.googlesource.gerrit.plugins.quota;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class MinStartForQueueQuota {
public static final Logger log = LoggerFactory.getLogger(MinStartForQueueQuota.class);
+ public static final String KEY = "minStartForQueue";
// 10 SSH-Interactive-Worker
public static final Pattern CONFIG_PATTERN = Pattern.compile("(\\d+)\\s+(.+)");
@@ -35,16 +35,17 @@
}
if (matcher.matches()) {
- int limit = Integer.parseInt(matcher.group(1));
+ int reservation = Integer.parseInt(matcher.group(1));
String queue = matcher.group(2);
QueueManager.registerReservation(
queue,
new QueueManager.Reservation(
- limit,
+ reservation,
task -> {
return task.getQueueName().equalsIgnoreCase(queue)
&& TaskQuotas.estimateProject(task).map(qs::matches).orElse(false);
- }));
+ },
+ qs.getNamespace()));
} else {
log.error("Invalid configuration entry [{}]", cfg);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/Module.java b/src/main/java/com/googlesource/gerrit/plugins/quota/Module.java
index 781787a..045801f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/Module.java
@@ -96,6 +96,9 @@
bind(LifecycleListener.class)
.annotatedWith(UniqueAnnotations.create())
.to(PublisherScheduler.class);
+ bind(LifecycleListener.class)
+ .annotatedWith(UniqueAnnotations.create())
+ .to(TaskQuotaLogFile.class);
DynamicSet.bind(binder(), UploadValidationListener.class).to(RateLimitUploadListener.class);
bindConstant()
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/NamespacedQuotaSection.java b/src/main/java/com/googlesource/gerrit/plugins/quota/NamespacedQuotaSection.java
index ad8a90c..7e7b516 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/NamespacedQuotaSection.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/NamespacedQuotaSection.java
@@ -47,9 +47,4 @@
public String subSection() {
return namespace();
}
-
- @Override
- public boolean isFallbackQuota() {
- return isFallBack;
- }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/ParkedQuotaTransitionLogger.java b/src/main/java/com/googlesource/gerrit/plugins/quota/ParkedQuotaTransitionLogger.java
new file mode 100644
index 0000000..4377188
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/ParkedQuotaTransitionLogger.java
@@ -0,0 +1,109 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.quota;
+
+import static com.google.gerrit.server.git.WorkQueue.Task;
+import static com.googlesource.gerrit.plugins.quota.QueueManager.Queue;
+import static com.googlesource.gerrit.plugins.quota.QueueManager.QueueInfo;
+
+import com.google.gerrit.server.ioutil.HexFormat;
+import com.google.gerrit.util.logging.NamedFluentLogger;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class ParkedQuotaTransitionLogger {
+ protected static final NamedFluentLogger quotaLog =
+ NamedFluentLogger.forName(TaskQuotaLogFile.NAME);
+ protected static final Map<Integer, TaskQuota> prevParkingQuotaByTaskId =
+ new ConcurrentHashMap<>();
+ protected static final Map<Integer, Instant> parkedSince = new ConcurrentHashMap<>();
+ protected static final TaskQuota CANNOT_SATISFY_RESERVATIONS_QUOTA =
+ new TaskQuota() {
+ @Override
+ public boolean isApplicable(Task<?> task) {
+ throw new IllegalStateException();
+ }
+
+ @Override
+ public boolean isReadyToStart(Task<?> task) {
+ throw new IllegalStateException();
+ }
+
+ @Override
+ public void onStop(Task<?> task) {
+ throw new IllegalStateException();
+ }
+ };
+
+ /** Logs only if the reason for parked changes from the previous parking event. */
+ public static void logTaskWithEnforcedQuota(Task<?> t, TaskQuota q) {
+ parkedSince.putIfAbsent(t.getTaskId(), Instant.now());
+ if (shouldLog(t, q)) {
+ quotaLog.atInfo().log("Task [%s] parked due to quota rule [%s]", formatTask(t), q);
+ }
+ }
+
+ public static void logTaskWithNoSatisfyingReservation(Task<?> t) {
+ parkedSince.putIfAbsent(t.getTaskId(), Instant.now());
+ if (!shouldLog(t, CANNOT_SATISFY_RESERVATIONS_QUOTA)) {
+ return;
+ }
+
+ QueueInfo queueInfo = QueueManager.infoByQueue.get(Queue.fromKey(t.getQueueName()));
+ if (queueInfo != null) {
+ queueInfo.reservations.stream()
+ .filter(r -> r.matches(t))
+ .findFirst()
+ .ifPresentOrElse(
+ reservation -> {
+ quotaLog.atInfo().log(
+ "Task [%s] parked because there are no spare unreserved threads in queue [%s], "
+ + "and there are insufficient reserved threads for the %s namespace",
+ formatTask(t), t.getQueueName(), reservation.namespace());
+ },
+ () -> {
+ quotaLog.atInfo().log(
+ "Task [%s] parked because there are no spare unreserved threads in queue [%s]",
+ formatTask(t), t.getQueueName());
+ });
+ }
+ }
+
+ public static void logOnTaskStartIfParked(Task<?> t) {
+ if (!prevParkingQuotaByTaskId.containsKey(t.getTaskId())) {
+ return;
+ }
+
+ quotaLog.atInfo().log(
+ "Task [%s] unparked after %d seconds",
+ formatTask(t), Duration.between(parkedSince.get(t.getTaskId()), Instant.now()).toSeconds());
+ clear(t);
+ }
+
+ public static boolean shouldLog(Task<?> t, TaskQuota q) {
+ return prevParkingQuotaByTaskId.put(t.getTaskId(), q) != q;
+ }
+
+ public static void clear(Task<?> t) {
+ prevParkingQuotaByTaskId.remove(t.getTaskId());
+ parkedSince.remove(t.getTaskId());
+ }
+
+ public static String formatTask(Task<?> t) {
+ return "%s: [%s]".formatted(HexFormat.fromInt(t.getTaskId()), t);
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/QueueManager.java b/src/main/java/com/googlesource/gerrit/plugins/quota/QueueManager.java
index 4b0010d..a9ffb10 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/QueueManager.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/QueueManager.java
@@ -15,19 +15,17 @@
package com.googlesource.gerrit.plugins.quota;
import com.google.gerrit.server.git.WorkQueue;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Predicate;
-
-import java.util.*;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Predicate;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class QueueManager {
public static final Logger log = LoggerFactory.getLogger(QueueManager.class);
@@ -101,7 +99,8 @@
}
}
- public record Reservation(int reservedCapacity, Predicate<WorkQueue.Task<?>> taskMatcher) {
+ public record Reservation(
+ int reservedCapacity, Predicate<WorkQueue.Task<?>> taskMatcher, String namespace) {
public boolean matches(WorkQueue.Task<?> task) {
return taskMatcher.test(task);
}
@@ -164,7 +163,8 @@
qName,
reservation.reservedCapacity(),
capacityToReserve);
- queueInfo.addReservation(new Reservation(capacityToReserve, reservation.taskMatcher));
+ queueInfo.addReservation(
+ new Reservation(capacityToReserve, reservation.taskMatcher, reservation.namespace()));
return;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/QuotaFinder.java b/src/main/java/com/googlesource/gerrit/plugins/quota/QuotaFinder.java
index 44d27c1..9d2d27b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/QuotaFinder.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/QuotaFinder.java
@@ -69,7 +69,7 @@
}
public QuotaSection getFallbackNamespacedQuota(Config cfg) {
- return new NamespacedQuotaSection(cfg, "*", true);
+ return new NamespacedQuotaSection(cfg, "*");
}
public List<NamespacedQuotaSection> getQuotaNamespaces(Config cfg) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/SoftMaxPerUserForQueue.java b/src/main/java/com/googlesource/gerrit/plugins/quota/SoftMaxPerUserForQueue.java
index de7c630..19fa018 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/SoftMaxPerUserForQueue.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/SoftMaxPerUserForQueue.java
@@ -24,14 +24,17 @@
import java.util.regex.Pattern;
public class SoftMaxPerUserForQueue implements TaskQuota {
+ public static final String KEY = "softMaxStartPerUserForQueue";
public static final Pattern CONFIG_PATTERN =
Pattern.compile("(\\d+)\\s+(" + String.join("|", QueueManager.Queue.keys()) + ")");
+ private final QuotaSection quotaSection;
private final int softMax;
private final QueueManager.Queue queue;
private final ConcurrentHashMap<String, Integer> taskStartedCountByUser =
new ConcurrentHashMap<>();
- public SoftMaxPerUserForQueue(int softMax, String queueName) {
+ public SoftMaxPerUserForQueue(QuotaSection quotaSection, int softMax, String queueName) {
+ this.quotaSection = quotaSection;
this.softMax = softMax;
this.queue = QueueManager.Queue.fromKey(queueName);
}
@@ -79,7 +82,14 @@
Matcher matcher = CONFIG_PATTERN.matcher(cfg);
return matcher.find()
? Optional.of(
- new SoftMaxPerUserForQueue(Integer.parseInt(matcher.group(1)), matcher.group(2)))
+ new SoftMaxPerUserForQueue(qs, Integer.parseInt(matcher.group(1)), matcher.group(2)))
: Optional.empty();
}
+
+ @Override
+ public String toString() {
+ return KEY
+ + ": softMax [%d], queue [%s], namespace [%s]"
+ .formatted(softMax, queue.getName(), quotaSection.getNamespace());
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskParser.java b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskParser.java
index 732b082..5944308 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskParser.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskParser.java
@@ -20,7 +20,7 @@
import java.util.regex.Pattern;
public class TaskParser {
- public static final Pattern USER_EXTRACT_PATTERN = Pattern.compile("\\(([a-z0-9]+)\\)$");
+ public static final Pattern USER_EXTRACT_PATTERN = Pattern.compile("\\(([\\-_A-Za-z0-9]+)\\)$");
public static Optional<String> user(WorkQueue.Task<?> task) {
Matcher matcher = USER_EXTRACT_PATTERN.matcher(task.toString());
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTask.java b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTask.java
index 2d15abe..8375c3b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTask.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTask.java
@@ -21,7 +21,7 @@
public abstract class TaskQuotaForTask extends TaskQuotaWithPermits {
protected static final Map<String, Set<String>> SUPPORTED_TASKS_BY_GROUP =
Map.of("uploadpack", Set.of("git-upload-pack"), "receivepack", Set.of("git-receive-pack"));
- private final String taskGroup;
+ public final String taskGroup;
public TaskQuotaForTask(String taskGroup, int permits) {
super(permits);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTaskForQueue.java b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTaskForQueue.java
index ac57a00..535edbf 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTaskForQueue.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTaskForQueue.java
@@ -23,13 +23,17 @@
public class TaskQuotaForTaskForQueue extends TaskQuotaForTask {
public static final Logger log = LoggerFactory.getLogger(TaskQuotaForTaskForQueue.class);
+ public static final String KEY = "maxStartForTaskForQueue";
public static final Pattern CONFIG_PATTERN =
Pattern.compile(
"(\\d+)\\s+(" + String.join("|", SUPPORTED_TASKS_BY_GROUP.keySet()) + ")\\s+(.+)");
- private final String queueName;
+ public final String queueName;
+ protected final QuotaSection quotaSection;
- public TaskQuotaForTaskForQueue(String queueName, String taskGroup, int maxStart) {
+ public TaskQuotaForTaskForQueue(
+ QuotaSection quotaSection, String queueName, String taskGroup, int maxStart) {
super(taskGroup, maxStart);
+ this.quotaSection = quotaSection;
this.queueName = queueName;
}
@@ -43,10 +47,17 @@
if (matcher.matches()) {
return Optional.of(
new TaskQuotaForTaskForQueue(
- matcher.group(3), matcher.group(2), Integer.parseInt(matcher.group(1))));
+ qs, matcher.group(3), matcher.group(2), Integer.parseInt(matcher.group(1))));
} else {
log.error("Invalid configuration entry [{}]", cfg);
return Optional.empty();
}
}
+
+ @Override
+ public String toString() {
+ return KEY
+ + ": task [%s], queue [%s], permits [%d], namespace [%s]"
+ .formatted(taskGroup, queueName, maxPermits, quotaSection.getNamespace());
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTaskForQueueForUser.java b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTaskForQueueForUser.java
index 7afa870..751559e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTaskForQueueForUser.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaForTaskForQueueForUser.java
@@ -23,6 +23,7 @@
public class TaskQuotaForTaskForQueueForUser extends TaskQuotaForTaskForQueue {
public static final Logger log = LoggerFactory.getLogger(TaskQuotaForTaskForQueueForUser.class);
+ public static final String KEY = "maxStartForTaskForUserForQueue";
public static final Pattern CONFIG_PATTERN =
Pattern.compile(
"(\\d+)\\s+("
@@ -33,8 +34,8 @@
private final String user;
public TaskQuotaForTaskForQueueForUser(
- String queueName, String user, String taskGroup, int maxStart) {
- super(queueName, taskGroup, maxStart);
+ QuotaSection quotaSection, String queueName, String user, String taskGroup, int maxStart) {
+ super(quotaSection, queueName, taskGroup, maxStart);
this.user = user;
}
@@ -49,6 +50,7 @@
if (matcher.matches()) {
return Optional.of(
new TaskQuotaForTaskForQueueForUser(
+ qs,
matcher.group(4),
matcher.group(3),
matcher.group(2),
@@ -58,4 +60,11 @@
return Optional.empty();
}
}
+
+ @Override
+ public String toString() {
+ return KEY
+ + ": task [%s], queue [%s], user [%s], permits [%d], namespace [%s]"
+ .formatted(taskGroup, queueName, user, maxPermits, quotaSection.getNamespace());
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaKeys.java b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaKeys.java
index ff0a4cb..65da2a5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaKeys.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaKeys.java
@@ -18,13 +18,13 @@
import java.util.function.BiFunction;
public enum TaskQuotaKeys {
- MAX_START_FOR_TASK_FOR_QUEUE("maxStartForTaskForQueue", TaskQuotaForTaskForQueue::build),
- MIN_START_FOR_TASK_FOR_QUEUE("minStartForQueue", MinStartForQueueQuota::build),
+ MAX_START_FOR_TASK_FOR_QUEUE(TaskQuotaForTaskForQueue.KEY, TaskQuotaForTaskForQueue::build),
+ MIN_START_FOR_TASK_FOR_QUEUE(MinStartForQueueQuota.KEY, MinStartForQueueQuota::build),
MAX_START_FOR_TASK_FOR_USER_FOR_QUEUE(
- "maxStartForTaskForUserForQueue", TaskQuotaForTaskForQueueForUser::build),
+ TaskQuotaForTaskForQueueForUser.KEY, TaskQuotaForTaskForQueueForUser::build),
MAX_START_PER_USER_FOR_TASK_FOR_QUEUE(
- "maxStartPerUserForTaskForQueue", TaskQuotaPerUserForTaskForQueue::build),
- SOFT_MAX_START_FOR_QUEUE_PER_USER("softMaxStartPerUserForQueue", SoftMaxPerUserForQueue::build);
+ TaskQuotaPerUserForTaskForQueue.KEY, TaskQuotaPerUserForTaskForQueue::build),
+ SOFT_MAX_START_FOR_QUEUE_PER_USER(SoftMaxPerUserForQueue.KEY, SoftMaxPerUserForQueue::build);
public final String key;
public final BiFunction<QuotaSection, String, Optional<TaskQuota>> processor;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaLogFile.java b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaLogFile.java
new file mode 100644
index 0000000..66c43ff
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaLogFile.java
@@ -0,0 +1,30 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.plugins.quota;
+
+import com.google.gerrit.extensions.systemstatus.ServerInformation;
+import com.google.gerrit.server.util.PluginLogFile;
+import com.google.gerrit.server.util.SystemLog;
+import com.google.inject.Inject;
+import org.apache.log4j.PatternLayout;
+
+public class TaskQuotaLogFile extends PluginLogFile {
+ protected static String NAME = "quota_log";
+
+ @Inject
+ public TaskQuotaLogFile(SystemLog systemLog, ServerInformation serverInfo) {
+ super(systemLog, serverInfo, NAME, new PatternLayout("[%d] %m%n"));
+ }
+}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaPerUserForTaskForQueue.java b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaPerUserForTaskForQueue.java
index a7299a2..dc2ee64 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaPerUserForTaskForQueue.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaPerUserForTaskForQueue.java
@@ -22,10 +22,12 @@
public class TaskQuotaPerUserForTaskForQueue extends TaskQuotaForTaskForQueue {
public static final Logger log = LoggerFactory.getLogger(TaskQuotaPerUserForTaskForQueue.class);
+ public static final String KEY = "maxStartPerUserForTaskForQueue";
private final PerUserTaskQuota perUserTaskQuota;
- public TaskQuotaPerUserForTaskForQueue(String queue, String taskGroup, int maxStart) {
- super(queue, taskGroup, maxStart);
+ public TaskQuotaPerUserForTaskForQueue(
+ QuotaSection quotaSection, String queue, String taskGroup, int maxStart) {
+ super(quotaSection, queue, taskGroup, maxStart);
perUserTaskQuota = new PerUserTaskQuota(maxStart);
}
@@ -44,10 +46,17 @@
if (matcher.matches()) {
return Optional.of(
new TaskQuotaPerUserForTaskForQueue(
- matcher.group(3), matcher.group(2), Integer.parseInt(matcher.group(1))));
+ qs, matcher.group(3), matcher.group(2), Integer.parseInt(matcher.group(1))));
} else {
log.error("Invalid configuration entry [{}]", cfg);
return Optional.empty();
}
}
+
+ @Override
+ public String toString() {
+ return KEY
+ + ": task [%s], queue [%s], permits [%d], namespace [%s]"
+ .formatted(taskGroup, queueName, maxPermits, quotaSection.getNamespace());
+ }
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaWithPermits.java b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaWithPermits.java
index ea414ca..e4c6ade 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaWithPermits.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotaWithPermits.java
@@ -1,3 +1,17 @@
+// Copyright (C) 2025 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
package com.googlesource.gerrit.plugins.quota;
import com.google.gerrit.server.git.WorkQueue;
@@ -5,9 +19,11 @@
public abstract class TaskQuotaWithPermits implements TaskQuota {
protected final AtomicInteger permits;
+ protected final int maxPermits;
public TaskQuotaWithPermits(int maxPermits) {
this.permits = new AtomicInteger(maxPermits);
+ this.maxPermits = maxPermits;
}
public boolean isReadyToStart(WorkQueue.Task<?> task) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotas.java b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotas.java
index 63a8bc1..41ea1c3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotas.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/TaskQuotas.java
@@ -19,7 +19,11 @@
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.ThreadSettingsConfig;
import com.google.gerrit.server.git.WorkQueue;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import java.util.regex.Matcher;
@@ -28,12 +32,9 @@
import javax.inject.Inject;
import javax.inject.Singleton;
import org.eclipse.jgit.lib.Config;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
public class TaskQuotas implements WorkQueue.TaskParker {
- private static final Logger log = LoggerFactory.getLogger(TaskQuotas.class);
private final QuotaFinder quotaFinder;
private final Map<Integer, List<TaskQuota>> quotasByTask = new ConcurrentHashMap<>();
private final Map<QuotaSection, List<TaskQuota>> quotasByNamespace = new HashMap<>();
@@ -83,8 +84,8 @@
@Override
public boolean isReadyToStart(WorkQueue.Task<?> task) {
- QueueManager.Queue queue = QueueManager.Queue.fromKey(task.getQueueName());
if (!QueueManager.acquire(task)) {
+ ParkedQuotaTransitionLogger.logTaskWithNoSatisfyingReservation(task);
return false;
}
@@ -105,7 +106,7 @@
for (TaskQuota quota : applicableQuotas) {
if (quota.isApplicable(task)) {
if (!quota.isReadyToStart(task)) {
- log.debug("Task [{}] will be parked due task quota rules", task);
+ ParkedQuotaTransitionLogger.logTaskWithEnforcedQuota(task, quota);
QueueManager.release(task);
acquiredQuotas.forEach(q -> q.onStop(task));
return false;
@@ -117,12 +118,15 @@
if (!acquiredQuotas.isEmpty()) {
quotasByTask.put(task.getTaskId(), acquiredQuotas);
}
+
+ ParkedQuotaTransitionLogger.logOnTaskStartIfParked(task);
return true;
}
@Override
public void onNotReadyToStart(WorkQueue.Task<?> task) {
QueueManager.release(task);
+ ParkedQuotaTransitionLogger.clear(task);
Optional.ofNullable(quotasByTask.remove(task.getTaskId()))
.ifPresent(quotas -> quotas.forEach(q -> q.onStop(task)));
}
@@ -133,6 +137,7 @@
@Override
public void onStop(WorkQueue.Task<?> task) {
QueueManager.release(task);
+ ParkedQuotaTransitionLogger.clear(task);
Optional.ofNullable(quotasByTask.remove(task.getTaskId()))
.ifPresent(quotas -> quotas.forEach(q -> q.onStop(task)));
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/quota/QueueManagerTest.java b/src/test/java/com/googlesource/gerrit/plugins/quota/QueueManagerTest.java
index 1740561..2e247e2 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/quota/QueueManagerTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/quota/QueueManagerTest.java
@@ -14,12 +14,16 @@
package com.googlesource.gerrit.plugins.quota;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.server.git.WorkQueue;
import com.googlesource.gerrit.plugins.quota.QueueManager.Queue;
import com.googlesource.gerrit.plugins.quota.QueueManager.QueueInfo;
-import com.google.gerrit.server.git.WorkQueue;
-import org.junit.Before;
-import org.junit.Test;
-
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
@@ -27,10 +31,8 @@
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.stream.IntStream;
-
-import static org.junit.Assert.*;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import org.junit.Before;
+import org.junit.Test;
public class QueueManagerTest {
private static final Queue TEST_QUEUE = Queue.INTERACTIVE;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/quota/TaskQuotasTest.java b/src/test/java/com/googlesource/gerrit/plugins/quota/TaskQuotasTest.java
index 05d23d2..61614c8 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/quota/TaskQuotasTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/quota/TaskQuotasTest.java
@@ -14,10 +14,12 @@
package com.googlesource.gerrit.plugins.quota;
-import static com.googlesource.gerrit.plugins.quota.QueueManager.Queue.*;
+import static com.googlesource.gerrit.plugins.quota.QueueManager.Queue.INTERACTIVE;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
import com.google.gerrit.server.git.WorkQueue.Task;
import java.util.Random;
@@ -32,6 +34,7 @@
public class TaskQuotasTest {
private static final String PROJECT_X = "project-x";
private static final String USER_A = "USER_A";
+ private static final String USER_B = "USER_B";
@Test
public void testMaxStartForTaskForQueue() throws ConfigInvalidException {
@@ -39,7 +42,7 @@
taskQuotas(
2,
2,
-"""
+ """
[quota "%s"]
maxStartForTaskForQueue = 1 uploadpack %s
"""
@@ -74,6 +77,61 @@
startAndCompleteTask(taskQuotas, u_x_3);
}
+ @Test
+ public void testSoftMaxPerUserForQueue() throws ConfigInvalidException {
+ TaskQuotas taskQuotas =
+ taskQuotas(
+ 5,
+ 5,
+ """
+[quota "%s"]
+ softMaxStartPerUserForQueue = 2 %s
+"""
+ .formatted(PROJECT_X, INTERACTIVE.getName()));
+
+ // running: user_a: 1 user_b: 0
+ Task<?> u_x_a_1 = task(INTERACTIVE.getName(), uploadPackTask(PROJECT_X, USER_A));
+ assertTrue(taskQuotas.isReadyToStart(u_x_a_1));
+ taskQuotas.onStart(u_x_a_1);
+
+ // running: user_a: 2 user_b: 0 (user_a is at the softMax)
+ Task<?> u_x_a_2 = task(INTERACTIVE.getName(), uploadPackTask(PROJECT_X, USER_A));
+ assertTrue(taskQuotas.isReadyToStart(u_x_a_2));
+ taskQuotas.onStart(u_x_a_2);
+
+ // running: user_a: 3 user_b: 0 (user_a able to start new task exceeding soft max)
+ Task<?> u_x_a_3 = task(INTERACTIVE.getName(), receivePackTask(PROJECT_X, USER_A));
+ assertTrue(taskQuotas.isReadyToStart(u_x_a_3));
+ taskQuotas.onStart(u_x_a_3);
+
+ // running: user_a: 3 user_b: 1
+ Task<?> u_x_b_1 = task(INTERACTIVE.getName(), uploadPackTask(PROJECT_X, USER_B));
+ assertTrue(taskQuotas.isReadyToStart(u_x_b_1));
+ taskQuotas.onStart(u_x_b_1);
+
+ // running: user_a: 3 user_b: 2
+ Task<?> u_x_b_2 = task(INTERACTIVE.getName(), receivePackTask(PROJECT_X, USER_B));
+ assertTrue(taskQuotas.isReadyToStart(u_x_b_2));
+ taskQuotas.onStart(u_x_b_2);
+
+ // running: user_a: 2 user_b: 2
+ taskQuotas.onStop(u_x_a_1);
+ Task<?> u_x_a_4 = task(INTERACTIVE.getName(), uploadPackTask(PROJECT_X, USER_A));
+ assertFalse(taskQuotas.isReadyToStart(u_x_a_4));
+
+ // running: user_a: 2 user_b: 1
+ taskQuotas.onStop(u_x_b_1);
+
+ // running: user_a: 3 user_b: 1
+ assertTrue(taskQuotas.isReadyToStart(u_x_a_4));
+ taskQuotas.onStart(u_x_a_4);
+
+ taskQuotas.onStop(u_x_a_2);
+ taskQuotas.onStop(u_x_a_3);
+ taskQuotas.onStop(u_x_a_4);
+ taskQuotas.onStop(u_x_b_2);
+ }
+
private Task<?> task(String queueName, String taskString) {
Task<?> task = Mockito.mock(Task.class);
when(task.getTaskId()).thenReturn(new Random().nextInt());
