Add readable text to the limit type
Add additional limitType parameter to enum RateLimitType class to
reuse in messages. This is done to improve log readability if the
plugin would support non-uploadpack rate-limiting in the future.
Feature: Issue 10301
Change-Id: I52058dae1c534d82eed82f4919d583c00395c2e0
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java
index c59c2fe..a856772 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java
@@ -36,6 +36,7 @@
class Module extends AbstractModule {
static final String UPLOAD_PACK_PER_HOUR = "upload_pack_per_hour";
+ static final String DEFAULT_RATE_LIMIT_TYPE = "upload pack";
@Override
protected void configure() {
@@ -96,10 +97,13 @@
return UnlimitedRateLimiter.INSTANCE;
}
+ String rateLimitType = DEFAULT_RATE_LIMIT_TYPE;
+
// In the case that there is a warning but no limit
Integer myLimit = Integer.MAX_VALUE;
if (limit.isPresent()) {
myLimit = limit.get().getRatePerHour();
+ rateLimitType = limit.get().getType().getLimitType();
}
long effectiveTimeLapse = PeriodicRateLimiter.DEFAULT_TIME_LAPSE_IN_MINUTES;
@@ -107,13 +111,15 @@
long providedTimeLapse = timeLapse.get().getRatePerHour();
if (providedTimeLapse > 0 && providedTimeLapse <= effectiveTimeLapse) {
effectiveTimeLapse = providedTimeLapse;
+ rateLimitType = timeLapse.get().getType().getLimitType();
} else {
logger.warn(
"The time lapse is set to the default {} minutes, as the configured value is invalid.",
effectiveTimeLapse);
}
}
- RateLimiter rateLimiter = periodicRateLimiterFactory.create(myLimit, effectiveTimeLapse);
+ RateLimiter rateLimiter =
+ periodicRateLimiterFactory.create(myLimit, effectiveTimeLapse, rateLimitType);
if (warn.isPresent()) {
if (limit.isPresent()) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java
index cb86336..5c5ced3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java
@@ -29,19 +29,22 @@
private final int maxPermits;
private final AtomicInteger usedPermits;
private final ScheduledFuture<?> replenishTask;
+ private final String rateLimitType;
interface Factory {
- PeriodicRateLimiter create(int permits, long timeLapse);
+ PeriodicRateLimiter create(int permits, long timeLapse, String rateLimitType);
}
@Inject
PeriodicRateLimiter(
@RateLimitExecutor ScheduledExecutorService executor,
@Assisted int permits,
- @Assisted long timeLapse) {
+ @Assisted long timeLapse,
+ @Assisted String rateLimitType) {
this.semaphore = new Semaphore(permits);
this.maxPermits = permits;
this.usedPermits = new AtomicInteger();
+ this.rateLimitType = rateLimitType;
this.replenishTask =
executor.scheduleAtFixedRate(
this::replenishPermits, timeLapse, timeLapse, TimeUnit.MINUTES);
@@ -83,6 +86,11 @@
}
@Override
+ public String getType() {
+ return rateLimitType;
+ }
+
+ @Override
public void close() {
replenishTask.cancel(true);
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitType.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitType.java
index 337508f..0d32776 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitType.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitType.java
@@ -15,14 +15,16 @@
package com.googlesource.gerrit.plugins.ratelimiter;
enum RateLimitType {
- UPLOAD_PACK_PER_HOUR("uploadpackperhour"),
- UPLOAD_PACK_PER_HOUR_WARN("uploadpackperhourwarn"),
- TIME_LAPSE_IN_MINUTES("timelapseinminutes");
+ UPLOAD_PACK_PER_HOUR("uploadpackperhour", "upload pack"),
+ UPLOAD_PACK_PER_HOUR_WARN("uploadpackperhourwarn", "upload pack"),
+ TIME_LAPSE_IN_MINUTES("timelapseinminutes", "upload pack");
private final String type;
+ private final String limitType;
- RateLimitType(String type) {
+ RateLimitType(String type, String limitType) {
this.type = type;
+ this.limitType = limitType;
}
@Override
@@ -30,6 +32,10 @@
return type;
}
+ public String getLimitType() {
+ return limitType;
+ }
+
static RateLimitType from(String value) {
for (RateLimitType rateLimitType : RateLimitType.values()) {
if (rateLimitType.toString().equalsIgnoreCase(value)) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiter.java
index 7c695de..d5455b1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiter.java
@@ -46,6 +46,9 @@
/** Replenish available permits to the number allowed per hour. */
void replenishPermits();
+ /** Return type of rate limiter * */
+ String getType();
+
/** Closes this RateLimiter, relinquishing any underlying resources. */
void close();
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UnlimitedRateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UnlimitedRateLimiter.java
index a3e682e..d1989f4 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UnlimitedRateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UnlimitedRateLimiter.java
@@ -14,6 +14,8 @@
package com.googlesource.gerrit.plugins.ratelimiter;
+import static com.googlesource.gerrit.plugins.ratelimiter.Module.DEFAULT_RATE_LIMIT_TYPE;
+
import java.util.concurrent.TimeUnit;
class UnlimitedRateLimiter implements RateLimiter {
@@ -48,6 +50,11 @@
}
@Override
+ public String getType() {
+ return DEFAULT_RATE_LIMIT_TYPE;
+ }
+
+ @Override
public int usedPermits() {
return 0;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiter.java
index 5ce2200..317cd26 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiter.java
@@ -64,19 +64,21 @@
boolean acquirePermit = delegate.acquirePermit();
if (usedPermits() == warnLimit) {
rateLimitLog.info(
- "{} reached the warning limit of {} uploadpacks per {} minutes.",
+ "{} reached the warning limit of {} {} per {} minutes.",
userResolver.getUserName(key).orElse(key),
warnLimit,
+ delegate.getType(),
timeLapse);
warningWasLogged = true;
}
if (!acquirePermit && !wasLogged) {
rateLimitLog.info(
- "{} was blocked due to exceeding the limit of {} uploadpacks per {} minutes."
+ "{} was blocked due to exceeding the limit of {} {} per {} minutes."
+ " {} remaining to permits replenishing.",
userResolver.getUserName(key).orElse(key),
permitsPerHour(),
+ delegate.getType(),
timeLapse,
secondsToMsSs(remainingTime(TimeUnit.SECONDS)));
wasLogged = true;
@@ -101,6 +103,11 @@
}
@Override
+ public String getType() {
+ return delegate.getType();
+ }
+
+ @Override
public void close() {
delegate.close();
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiter.java
index f9e55b3..7ce5be1 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiter.java
@@ -61,9 +61,10 @@
if (acquirePermit && (usedPermits() == warnLimit)) {
rateLimitLog.info(
- "{} reached the warning limit of {} uploadpacks per {} minutes.",
+ "{} reached the warning limit of {} {} per {} minutes.",
userResolver.getUserName(key).orElse(key),
warnLimit,
+ delegate.getType(),
timeLapse);
warningWasLogged = true;
}
@@ -87,6 +88,11 @@
}
@Override
+ public String getType() {
+ return delegate.getType();
+ }
+
+ @Override
public int usedPermits() {
return delegate.usedPermits();
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiterTest.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiterTest.java
index 1cb3764..b949490 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiterTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiterTest.java
@@ -37,7 +37,9 @@
@Before
public void setUp() {
scheduledExecutorMock = mock(ScheduledExecutorService.class);
- limiter = new PeriodicRateLimiter(scheduledExecutorMock, RATE, DEFAULT_TIME_LAPSE_IN_MINUTES);
+ limiter =
+ new PeriodicRateLimiter(
+ scheduledExecutorMock, RATE, DEFAULT_TIME_LAPSE_IN_MINUTES, "Any Type");
}
@Test
diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiterTest.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiterTest.java
index 0353a92..9b41949 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiterTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiterTest.java
@@ -45,11 +45,15 @@
ScheduledExecutorService scheduledExecutorMock2 = mock(ScheduledExecutorService.class);
PeriodicRateLimiter limiter1 =
- spy(new PeriodicRateLimiter(scheduledExecutorMock1, RATE, DEFAULT_TIME_LAPSE_IN_MINUTES));
+ spy(
+ new PeriodicRateLimiter(
+ scheduledExecutorMock1, RATE, DEFAULT_TIME_LAPSE_IN_MINUTES, "Any Type"));
doReturn(1L).when(limiter1).remainingTime(any(TimeUnit.class));
PeriodicRateLimiter limiter2 =
- spy(new PeriodicRateLimiter(scheduledExecutorMock2, RATE, DEFAULT_TIME_LAPSE_IN_MINUTES));
+ spy(
+ new PeriodicRateLimiter(
+ scheduledExecutorMock2, RATE, DEFAULT_TIME_LAPSE_IN_MINUTES, "Any Type"));
doReturn(1L).when(limiter2).remainingTime(any(TimeUnit.class));
warningLimiter1 =
diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiterTest.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiterTest.java
index facabbe..7717234 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiterTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiterTest.java
@@ -39,7 +39,8 @@
public void setUp() {
scheduledExecutorMock = mock(ScheduledExecutorService.class);
PeriodicRateLimiter limiter =
- new PeriodicRateLimiter(scheduledExecutorMock, RATE, DEFAULT_TIME_LAPSE_IN_MINUTES);
+ new PeriodicRateLimiter(
+ scheduledExecutorMock, RATE, DEFAULT_TIME_LAPSE_IN_MINUTES, "Any Type");
warningUnlimitedLimiter =
new WarningUnlimitedRateLimiter(
userResolver, limiter, "dummy", WARN_RATE, DEFAULT_TIME_LAPSE_IN_MINUTES);
