Merge branch 'stable-3.1' into stable-3.2 * stable-3.1: fixup!: Fix omitted documentation fixup!: Fix omitted import plugins and libraries Display Used Permits in addition to permits available Add readable text to the limit type Change-Id: I64d23d6df67afd1a40b7ad5205faed907e492c6d
diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/ListCommand.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/ListCommand.java index 9f6c946..30442e9 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/ListCommand.java +++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/ListCommand.java
@@ -11,7 +11,6 @@ // 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.ratelimiter; import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER_OR_SLAVE; @@ -38,10 +37,9 @@ description = "Display rate limits statistics", runsAt = MASTER_OR_SLAVE) final class ListCommand extends SshCommand { - private static final String FORMAT = "%-26s %-17s %-19s %s"; + private static final String FORMAT = "%-26s %-17s %-19s %-15s %s"; private static final String DASHED_LINE = - "------------------------------------------------------------------------------"; - + "---------------------------------------------------------------------------------------------"; private final LoadingCache<String, RateLimiter> uploadPackPerHour; private final UserResolver userResolver; @@ -65,6 +63,7 @@ "Account Id/IP (username)", "Permits Per Hour", "Available Permits", + "Used Permits", "Replenish in")); stdout.println(DASHED_LINE); uploadPackPerHour.asMap().entrySet().stream() @@ -83,6 +82,7 @@ getDisplayValue(entry.getKey()), permits(entry.getValue().permitsPerHour()), permits(entry.getValue().availablePermits()), + permits(entry.getValue().usedPermits()), Duration.ofSeconds(entry.getValue().remainingTime(TimeUnit.SECONDS)))); }
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/main/resources/Documentation/cmd-list.md b/src/main/resources/Documentation/cmd-list.md index d541cfa..fcb7be8 100644 --- a/src/main/resources/Documentation/cmd-list.md +++ b/src/main/resources/Documentation/cmd-list.md
@@ -23,14 +23,13 @@ EXAMPLES -------- - > $ ssh -p @SSH_PORT@ @SSH_HOST@ @PLUGIN@ list -> ------------------------------------------------------------------------------ +> --------------------------------------------------------------------------------------------- > * upload_pack_per_hour * -> ------------------------------------------------------------------------------ -> Account Id (or IP) Permits Per Hour Available Permits Replenish in -> ------------------------------------------------------------------------------ -> 1000001 unlimited unlimited PT0S -> 1000002 1000 999 PT59M30S -> 127.0.0.1 1000 123 PT10M26S -> ------------------------------------------------------------------------------ +> --------------------------------------------------------------------------------------------- +> Account Id/IP (username) Permits Per Hour Available Permits Used Permits Replenish in +> --------------------------------------------------------------------------------------------- +> 1000000 (admin) unlimited unlimited 0 PT0S +> 1000001 (test_user) 1000 999 1 PT59M30S +> 127.0.0.1 1000 123 877 PT10M26S +> ---------------------------------------------------------------------------------------------
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);