Remove default rate limits
For both uploadpack and restapi rate limits remove defaults, so that
no configuration consistently means no imposed limits.
Change-Id: Iac8ca09950a11330f748592225578e2048246769
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsConfig.java b/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsConfig.java
index 591d1bb..81dbcd8 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/AccountLimitsConfig.java
@@ -35,8 +35,6 @@
import org.slf4j.LoggerFactory;
public class AccountLimitsConfig {
- private static final int DEFAULT_BURST_COUNT = 30;
- private static final int DEFAULT_INTERVAL_SECONDS = 60;
private static final Pattern PATTERN =
Pattern.compile("^\\s*(\\d+)\\s*/\\s*(.*)\\s*burst\\s*(\\d+)$");
private static final Logger log = LoggerFactory.getLogger(AccountLimitsConfig.class);
@@ -97,45 +95,44 @@
}
rateLimits = ArrayTable.create(Arrays.asList(Type.values()), groups);
for (String groupName : groups) {
- rateLimits.put(UPLOADPACK, groupName, parseRateLimit(c, groupName, UPLOADPACK));
- rateLimits.put(RESTAPI, groupName, parseRateLimit(c, groupName, RESTAPI));
+ parseRateLimit(c, groupName, UPLOADPACK);
+ parseRateLimit(c, groupName, RESTAPI);
}
}
- RateLimit parseRateLimit(Config c, String groupName, Type type) {
+ void parseRateLimit(Config c, String groupName, Type type) {
String name = type.toConfigValue();
String value = c.getString(GROUP_SECTION, groupName, name);
if (value == null) {
- return defaultRateLimit(type);
+ return;
}
value = value.trim();
Matcher m = PATTERN.matcher(value);
if (!m.matches()) {
- log.warn(
- "Invalid ''{}'' ratelimit configuration ''{}'', use default ratelimit {}/hour",
+ log.error(
+ "Invalid ''{}'' ratelimit configuration ''{}''; ignoring the configuration entry",
name,
- value,
- 3600.0D / DEFAULT_INTERVAL_SECONDS);
- return defaultRateLimit(type);
+ value);
+ return;
}
String digits = m.group(1);
String unitName = m.group(2).trim();
String storeCountString = m.group(3).trim();
- long burstCount = DEFAULT_BURST_COUNT;
+ long burstCount;
try {
burstCount = Long.parseLong(storeCountString);
} catch (NumberFormatException e) {
- log.warn(
- "Invalid ''{}'' ratelimit store configuration ''{}'', use default burst count ''{}''",
+ log.error(
+ "Invalid ''{}'' ratelimit store configuration ''{}''; ignoring the configuration entry",
name,
- storeCountString,
- burstCount);
+ storeCountString);
+ return;
}
TimeUnit inputUnit = TimeUnit.HOURS;
- double ratePerSecond = 1.0D / DEFAULT_INTERVAL_SECONDS;
+ double ratePerSecond;
if (match(unitName, "s", "sec", "second")) {
inputUnit = TimeUnit.SECONDS;
} else if (match(unitName, "m", "min", "minute")) {
@@ -146,15 +143,17 @@
inputUnit = TimeUnit.DAYS;
} else {
logNotRateUnit(GROUP_SECTION, groupName, name, value);
+ return;
}
try {
ratePerSecond = 1.0D * Long.parseLong(digits) / TimeUnit.SECONDS.convert(1, inputUnit);
} catch (NumberFormatException nfe) {
logNotRateUnit(GROUP_SECTION, groupName, unitName, value);
+ return;
}
int maxBurstSeconds = (int) (burstCount / ratePerSecond);
- return new RateLimit(type, ratePerSecond, maxBurstSeconds);
+ rateLimits.put(type, groupName, new RateLimit(type, ratePerSecond, maxBurstSeconds));
}
private static boolean match(final String a, final String... cases) {
@@ -170,18 +169,16 @@
if (subsection != null) {
log.error(
MessageFormat.format(
- "Invalid rate unit value: {0}.{1}.{2}={3}", section, subsection, name, valueString));
+ "Invalid rate unit value: {0}.{1}.{2}={3}; ignoring the configuration entry",
+ section, subsection, name, valueString));
} else {
log.error(
- MessageFormat.format("Invalid rate unit value: {0}.{1}={2}", section, name, valueString));
+ MessageFormat.format(
+ "Invalid rate unit value: {0}.{1}={2}; ignoring the configuration entry",
+ section, name, valueString));
}
}
- private RateLimit defaultRateLimit(Type type) {
- return new RateLimit(
- type, 1.0D / DEFAULT_INTERVAL_SECONDS, DEFAULT_INTERVAL_SECONDS * DEFAULT_BURST_COUNT);
- }
-
/**
* @param type type of rate limit
* @return map of rate limits per group name
diff --git a/src/main/java/com/googlesource/gerrit/plugins/quota/RateMsgHelper.java b/src/main/java/com/googlesource/gerrit/plugins/quota/RateMsgHelper.java
index a09ecfa..9640d06 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/quota/RateMsgHelper.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/quota/RateMsgHelper.java
@@ -26,7 +26,6 @@
private static final String UPLOADPACK_INLINE_NAME = "fetch";
private static final String RESTAPI_INLINE_NAME = "REST API";
- // compare AccountLimitsConfig's constructor for default rate limits
private static String getDefaultTemplateMsg(String rateLimitTypeName) {
return "Exceeded rate limit of "
+ RATE_LIMIT_TOKEN
diff --git a/src/main/resources/Documentation/config.md b/src/main/resources/Documentation/config.md
index 9a27a46..c8eaaea 100644
--- a/src/main/resources/Documentation/config.md
+++ b/src/main/resources/Documentation/config.md
@@ -199,7 +199,6 @@
`/m`, `/min`, `/minute`: requests per minute<br />
`/h`, `/hr`, `/hour`: requests per hour<br />
`/d`, `/day`: requests per day<br />
-The default unit used if no unit is configured is `/hour`.
<a id="burst" />
`group.<groupName>.<storedRequests>`
@@ -211,13 +210,7 @@
as if idle time would already have been accumulated.
If a rate limit configuration value is invalid or missing for a group,
-default values are assumed.
-
-For `uploadpack`, a default rate limit of 1
-request per minute with 30 stored requests is assumed.
-
-For `restapi`, the default rate limit is 20 requests per minute
-and at most 90 stored requests.
+the configuration entry gets ignored, and a warning is being logged.
Example:
