opendev/gerrit
Code Issues Proposed changes

Enforce repository size on pack rather than on object

Browse Source 
Issue:
Originally max repository size was enforced by the quota plugin.
However, that functionality was a perfect candidate for core quota
implementation and as such was implemented in [1].

As a result, Gerrit core is responsible for making sure that the limit
is enforced and the plugin is responsible for providing the limit.
Unfortunately, Gerrit core implementation developed a fault as it
enforced the limit on the object size instead of the pack size leading
to the situation where multiple smaller objects would fall under the
limit where the pack they belong to should already exhaust it.

Solution:
Enforce available repository size tokens on pack size (as it was
done this way originally in plugin).

[1] https://gerrit-review.googlesource.com/c/gerrit/+/224728

Bug: issue 14118
Change-Id: I37e573971a76abe53a3e6456498940a4bb803d0c
This commit is contained in:
Jacek Centkowski
2021-03-02 14:45:41 +01:00
parent 240cb38021
commit c99414ab4b
3 changed files with 8 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/com/google/gerrit/acceptance/InProcessProtocol.java
View File

@@ -337,7 +337,7 @@ class InProcessProtocol extends TestProtocol<Context> {
.project(req.project)
.availableTokens(REPOSITORY_SIZE_GROUP);
availableTokens.throwOnError();
availableTokens.availableTokens().ifPresent(v -> rp.setMaxObjectSizeLimit(v));
availableTokens.availableTokens().ifPresent(rp::setMaxPackSizeLimit);
ImmutableList<PostReceiveHook> hooks =
ImmutableList.<PostReceiveHook>builder()

2
java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
View File

@@ -327,7 +327,7 @@ public class AsyncReceiveCommits implements PreReceiveHook {
REPOSITORY_SIZE_GROUP, projectName);
throw new RuntimeException(e);
}
availableTokens.availableTokens().ifPresent(v -> receivePack.setMaxObjectSizeLimit(v));
availableTokens.availableTokens().ifPresent(receivePack::setMaxPackSizeLimit);
}
/** Determine if the user can upload commits. */

11
javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java
View File

@@ -34,7 +34,7 @@ import com.google.gerrit.server.quota.QuotaResponse;
import com.google.inject.Module;
import java.util.Collections;
import org.easymock.EasyMock;
import org.eclipse.jgit.api.errors.TooLargeObjectInPackException;
import org.eclipse.jgit.api.errors.TooLargePackException;
import org.eclipse.jgit.api.errors.TransportException;
import org.junit.Before;
import org.junit.Test;
@@ -77,7 +77,7 @@ public class RepositorySizeQuotaIT extends AbstractDaemonTest {
@Test
public void pushWithAvailableTokens() throws Exception {
expect(quotaBackendWithResource.availableTokens(REPOSITORY_SIZE_GROUP))
.andReturn(singletonAggregation(ok(276L)))
.andReturn(singletonAggregation(ok(277L)))
.times(2);
expect(quotaBackendWithResource.requestTokens(eq(REPOSITORY_SIZE_GROUP), anyLong()))
.andReturn(singletonAggregation(ok()));
@@ -101,11 +101,12 @@ public class RepositorySizeQuotaIT extends AbstractDaemonTest {
try {
pushCommit();
assert_().fail("expected TooLargeObjectInPackException");
} catch (TooLargeObjectInPackException e) {
} catch (TooLargePackException e) {
String msg = e.getMessage();
assertThat(msg).contains("Object too large");
assertThat(msg)
.contains(String.format("Max object size limit is %d bytes.", availableTokens));
.contains(
String.format(
"Pack exceeds the limit of %d bytes, rejecting the pack", availableTokens));
}
verify(quotaBackendWithUser);
verify(quotaBackendWithResource);