Merge "Merge branch 'stable-3.0'"

This commit is contained in:
David Pursehouse
2019-07-27 03:23:36 +00:00
committed by Gerrit Code Review
25 changed files with 560 additions and 28 deletions

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance;
import static com.google.common.truth.Truth.assertThat;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.LinkedListMultimap;
@@ -109,7 +110,8 @@ public class EventRecorder {
return events;
}
private ImmutableList<ChangeMergedEvent> getChangeMergedEvents(
@VisibleForTesting
public ImmutableList<ChangeMergedEvent> getChangeMergedEvents(
String project, String branch, int expectedSize) {
String key = refEventKey(ChangeMergedEvent.TYPE, project, branch);
if (expectedSize == 0) {

View File

@@ -14,6 +14,10 @@
package com.google.gerrit.acceptance;
import static com.google.gerrit.server.git.receive.LazyPostReceiveHookChain.affectsSize;
import static com.google.gerrit.server.quota.QuotaGroupDefinitions.REPOSITORY_SIZE_GROUP;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.InProcessProtocol.Context;
import com.google.gerrit.common.data.Capable;
@@ -40,6 +44,9 @@ import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.quota.QuotaBackend;
import com.google.gerrit.server.quota.QuotaException;
import com.google.gerrit.server.quota.QuotaResponse;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.RequestScopePropagator;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -261,6 +268,7 @@ class InProcessProtocol extends TestProtocol<Context> {
private final DynamicSet<PostReceiveHook> postReceiveHooks;
private final ThreadLocalRequestContext threadContext;
private final PermissionBackend permissionBackend;
private final QuotaBackend quotaBackend;
@Inject
Receive(
@@ -271,7 +279,8 @@ class InProcessProtocol extends TestProtocol<Context> {
PluginSetContext<ReceivePackInitializer> receivePackInitializers,
DynamicSet<PostReceiveHook> postReceiveHooks,
ThreadLocalRequestContext threadContext,
PermissionBackend permissionBackend) {
PermissionBackend permissionBackend,
QuotaBackend quotaBackend) {
this.userProvider = userProvider;
this.projectCache = projectCache;
this.factory = factory;
@@ -280,6 +289,7 @@ class InProcessProtocol extends TestProtocol<Context> {
this.postReceiveHooks = postReceiveHooks;
this.threadContext = threadContext;
this.permissionBackend = permissionBackend;
this.quotaBackend = quotaBackend;
}
@Override
@@ -319,10 +329,35 @@ class InProcessProtocol extends TestProtocol<Context> {
receivePackInitializers.runEach(
initializer -> initializer.init(projectState.getNameKey(), rp));
QuotaResponse.Aggregated availableTokens =
quotaBackend
.user(identifiedUser)
.project(req.project)
.availableTokens(REPOSITORY_SIZE_GROUP);
availableTokens.throwOnError();
availableTokens.availableTokens().ifPresent(v -> rp.setMaxObjectSizeLimit(v));
rp.setPostReceiveHook(PostReceiveHookChain.newChain(Lists.newArrayList(postReceiveHooks)));
ImmutableList<PostReceiveHook> hooks =
ImmutableList.<PostReceiveHook>builder()
.add(
(pack, commands) -> {
if (affectsSize(pack, commands)) {
try {
quotaBackend
.user(identifiedUser)
.project(req.project)
.requestTokens(REPOSITORY_SIZE_GROUP, pack.getPackSize())
.throwOnError();
} catch (QuotaException e) {
throw new RuntimeException(e);
}
}
})
.addAll(postReceiveHooks)
.build();
rp.setPostReceiveHook(PostReceiveHookChain.newChain(hooks));
return rp;
} catch (IOException | PermissionBackendException e) {
} catch (IOException | PermissionBackendException | QuotaException e) {
throw new RuntimeException(e);
}
}

View File

@@ -98,7 +98,7 @@ public class AccountCacheImpl implements AccountCache {
return byId.get(accountId);
} catch (ExecutionException e) {
logger.atWarning().withCause(e).log("Cannot load AccountState for ID %s", accountId);
return null;
return Optional.empty();
}
}
@@ -146,7 +146,7 @@ public class AccountCacheImpl implements AccountCache {
.orElseGet(Optional::empty);
} catch (IOException | ConfigInvalidException e) {
logger.atWarning().withCause(e).log("Cannot load AccountState for username %s", username);
return null;
return Optional.empty();
}
}

View File

@@ -59,7 +59,6 @@ public class NotifyResolver {
public abstract NotifyHandling handling();
// TODO(dborowitz): Should be ImmutableSetMultimap.
public abstract ImmutableSetMultimap<RecipientType, Account.Id> accounts();
public Result withHandling(NotifyHandling notifyHandling) {

View File

@@ -28,7 +28,7 @@ public class ThreadSettingsConfig {
@Inject
ThreadSettingsConfig(@GerritServerConfig Config cfg) {
int cores = Runtime.getRuntime().availableProcessors();
sshdThreads = cfg.getInt("sshd", "threads", 2 * cores);
sshdThreads = cfg.getInt("sshd", "threads", Math.max(4, 2 * cores));
httpdMaxThreads = cfg.getInt("httpd", "maxThreads", 25);
int defaultDatabasePoolLimit = sshdThreads + httpdMaxThreads + 2;
databasePoolLimit = cfg.getInt("database", "poolLimit", defaultDatabasePoolLimit);

View File

@@ -49,7 +49,10 @@ public class MergedByPushOp implements BatchUpdateOp {
public interface Factory {
MergedByPushOp create(
RequestScopePropagator requestScopePropagator, PatchSet.Id psId, String refName);
RequestScopePropagator requestScopePropagator,
PatchSet.Id psId,
@Assisted("refName") String refName,
@Assisted("mergeResultRevId") String mergeResultRevId);
}
private final RequestScopePropagator requestScopePropagator;
@@ -62,6 +65,7 @@ public class MergedByPushOp implements BatchUpdateOp {
private final PatchSet.Id psId;
private final String refName;
private final String mergeResultRevId;
private Change change;
private boolean correctBranch;
@@ -79,7 +83,8 @@ public class MergedByPushOp implements BatchUpdateOp {
ChangeMerged changeMerged,
@Assisted RequestScopePropagator requestScopePropagator,
@Assisted PatchSet.Id psId,
@Assisted String refName) {
@Assisted("refName") String refName,
@Assisted("mergeResultRevId") String mergeResultRevId) {
this.patchSetInfoFactory = patchSetInfoFactory;
this.cmUtil = cmUtil;
this.mergedSenderFactory = mergedSenderFactory;
@@ -89,6 +94,7 @@ public class MergedByPushOp implements BatchUpdateOp {
this.requestScopePropagator = requestScopePropagator;
this.psId = psId;
this.refName = refName;
this.mergeResultRevId = mergeResultRevId;
}
public String getMergedIntoRef() {
@@ -184,8 +190,7 @@ public class MergedByPushOp implements BatchUpdateOp {
}
}));
changeMerged.fire(
change, patchSet, ctx.getAccount(), patchSet.commitId().name(), ctx.getWhen());
changeMerged.fire(change, patchSet, ctx.getAccount(), mergeResultRevId, ctx.getWhen());
}
private PatchSetInfo getPatchSetInfo(ChangeContext ctx) throws IOException {

View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.git.receive;
import static com.google.gerrit.server.quota.QuotaGroupDefinitions.REPOSITORY_SIZE_GROUP;
import static java.util.concurrent.TimeUnit.NANOSECONDS;
import com.google.common.flogger.FluentLogger;
@@ -45,6 +46,9 @@ import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.project.ContributorAgreementsChecker;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.quota.QuotaBackend;
import com.google.gerrit.server.quota.QuotaException;
import com.google.gerrit.server.quota.QuotaResponse;
import com.google.gerrit.server.util.MagicBranch;
import com.google.gerrit.server.util.RequestScopePropagator;
import com.google.inject.Inject;
@@ -92,6 +96,7 @@ public class AsyncReceiveCommits implements PreReceiveHook {
public static class Module extends PrivateModule {
@Override
public void configure() {
install(new FactoryModuleBuilder().build(LazyPostReceiveHookChain.Factory.class));
install(new FactoryModuleBuilder().build(AsyncReceiveCommits.Factory.class));
expose(AsyncReceiveCommits.Factory.class);
// Don't expose the binding for ReceiveCommits.Factory. All callers should
@@ -257,9 +262,10 @@ public class AsyncReceiveCommits implements PreReceiveHook {
RequestScopePropagator scopePropagator,
ReceiveConfig receiveConfig,
TransferConfig transferConfig,
Provider<LazyPostReceiveHookChain> lazyPostReceive,
LazyPostReceiveHookChain.Factory lazyPostReceive,
ContributorAgreementsChecker contributorAgreements,
Metrics metrics,
QuotaBackend quotaBackend,
@Named(TIMEOUT_NAME) long timeoutMillis,
@Assisted ProjectState projectState,
@Assisted IdentifiedUser user,
@@ -288,7 +294,7 @@ public class AsyncReceiveCommits implements PreReceiveHook {
receivePack.setRefFilter(new ReceiveRefFilter());
receivePack.setAllowPushOptions(true);
receivePack.setPreReceiveHook(this);
receivePack.setPostReceiveHook(lazyPostReceive.get());
receivePack.setPostReceiveHook(lazyPostReceive.create(user, projectName));
// If the user lacks READ permission, some references may be filtered and hidden from view.
// Check objects mentioned inside the incoming pack file are reachable from visible refs.
@@ -310,6 +316,17 @@ public class AsyncReceiveCommits implements PreReceiveHook {
factory.create(
projectState, user, receivePack, allRefsWatcher, messageSender, resultChangeIds);
receiveCommits.init();
QuotaResponse.Aggregated availableTokens =
quotaBackend.user(user).project(projectName).availableTokens(REPOSITORY_SIZE_GROUP);
try {
availableTokens.throwOnError();
} catch (QuotaException e) {
logger.atWarning().withCause(e).log(
"Quota %s availableTokens request failed for project %s",
REPOSITORY_SIZE_GROUP, projectName);
throw new RuntimeException(e);
}
availableTokens.availableTokens().ifPresent(v -> receivePack.setMaxObjectSizeLimit(v));
}
/** Determine if the user can upload commits. */

View File

@@ -14,23 +14,78 @@
package com.google.gerrit.server.git.receive;
import static com.google.gerrit.server.quota.QuotaGroupDefinitions.REPOSITORY_SIZE_GROUP;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.quota.QuotaBackend;
import com.google.gerrit.server.quota.QuotaResponse;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.Collection;
import org.eclipse.jgit.transport.PostReceiveHook;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.transport.ReceivePack;
class LazyPostReceiveHookChain implements PostReceiveHook {
/**
* Class is responsible for calling all registered post-receive hooks. In addition, in case when
* repository size quota is defined, it requests tokens (pack size) that were received. This is the
* final step of enforcing repository size quota that deducts token from available tokens.
*/
public class LazyPostReceiveHookChain implements PostReceiveHook {
interface Factory {
LazyPostReceiveHookChain create(CurrentUser user, Project.NameKey project);
}
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final PluginSetContext<PostReceiveHook> hooks;
private final QuotaBackend quotaBackend;
private final CurrentUser user;
private final Project.NameKey project;
@Inject
LazyPostReceiveHookChain(PluginSetContext<PostReceiveHook> hooks) {
LazyPostReceiveHookChain(
PluginSetContext<PostReceiveHook> hooks,
QuotaBackend quotaBackend,
@Assisted CurrentUser user,
@Assisted Project.NameKey project) {
this.hooks = hooks;
this.quotaBackend = quotaBackend;
this.user = user;
this.project = project;
}
@Override
public void onPostReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
hooks.runEach(h -> h.onPostReceive(rp, commands));
if (affectsSize(rp, commands)) {
QuotaResponse.Aggregated a =
quotaBackend
.user(user)
.project(project)
.requestTokens(REPOSITORY_SIZE_GROUP, rp.getPackSize());
if (a.hasError()) {
String msg =
String.format(
"%s request failed for project %s with [%s]",
REPOSITORY_SIZE_GROUP, project, a.errorMessage());
logger.atWarning().log(msg);
throw new RuntimeException(msg);
}
}
}
public static boolean affectsSize(ReceivePack rp, Collection<ReceiveCommand> commands) {
if (rp.getPackSize() > 0L) {
for (ReceiveCommand cmd : commands) {
if (cmd.getType() != ReceiveCommand.Type.DELETE) {
return true;
}
}
}
return false;
}
}

View File

@@ -2952,6 +2952,7 @@ class ReceiveCommits {
projectState,
notes.getChange().getDest(),
checkMergedInto,
checkMergedInto ? inputCommand.getNewId().name() : null,
priorPatchSet,
priorCommit,
psId,
@@ -3187,6 +3188,9 @@ class ReceiveCommits {
int limit = receiveConfig.maxBatchCommits;
int n = 0;
for (RevCommit c; (c = walk.next()) != null; ) {
// Even if skipValidation is set, we still get here when at least one plugin
// commit validator requires to validate all commits. In this case, however,
// we don't need to check the commit limit.
if (++n > limit && !skipValidation) {
logger.atFine().log("Number of new commits exceeds limit of %d", limit);
reject(
@@ -3263,7 +3267,8 @@ class ReceiveCommits {
bu.addOp(notes.get().getChangeId(), setPrivateOpFactory.create(false, null));
bu.addOp(
psId.changeId(),
mergedByPushOpFactory.create(requestScopePropagator, psId, refName));
mergedByPushOpFactory.create(
requestScopePropagator, psId, refName, newTip.getId().getName()));
continue COMMIT;
}
}
@@ -3297,7 +3302,8 @@ class ReceiveCommits {
bu.addOp(
id,
mergedByPushOpFactory
.create(requestScopePropagator, req.psId, refName)
.create(
requestScopePropagator, req.psId, refName, newTip.getId().getName())
.setPatchSetProvider(req.replaceOp::getPatchSet));
bu.addOp(id, new ChangeProgressOp(progress));
ids.add(id);

View File

@@ -101,6 +101,7 @@ public class ReplaceOp implements BatchUpdateOp {
ProjectState projectState,
BranchNameKey dest,
boolean checkMergedInto,
@Nullable String mergeResultRevId,
@Assisted("priorPatchSetId") PatchSet.Id priorPatchSetId,
@Assisted("priorCommitId") ObjectId priorCommit,
@Assisted("patchSetId") PatchSet.Id patchSetId,
@@ -133,6 +134,7 @@ public class ReplaceOp implements BatchUpdateOp {
private final ProjectState projectState;
private final BranchNameKey dest;
private final boolean checkMergedInto;
private final String mergeResultRevId;
private final PatchSet.Id priorPatchSetId;
private final ObjectId priorCommitId;
private final PatchSet.Id patchSetId;
@@ -177,6 +179,7 @@ public class ReplaceOp implements BatchUpdateOp {
@Assisted ProjectState projectState,
@Assisted BranchNameKey dest,
@Assisted boolean checkMergedInto,
@Assisted @Nullable String mergeResultRevId,
@Assisted("priorPatchSetId") PatchSet.Id priorPatchSetId,
@Assisted("priorCommitId") ObjectId priorCommitId,
@Assisted("patchSetId") PatchSet.Id patchSetId,
@@ -205,6 +208,7 @@ public class ReplaceOp implements BatchUpdateOp {
this.projectState = projectState;
this.dest = dest;
this.checkMergedInto = checkMergedInto;
this.mergeResultRevId = mergeResultRevId;
this.priorPatchSetId = priorPatchSetId;
this.priorCommitId = priorCommitId.copy();
this.patchSetId = patchSetId;
@@ -231,7 +235,8 @@ public class ReplaceOp implements BatchUpdateOp {
String mergedInto = findMergedInto(ctx, dest.branch(), commit);
if (mergedInto != null) {
mergedByPushOp =
mergedByPushOpFactory.create(requestScopePropagator, patchSetId, mergedInto);
mergedByPushOpFactory.create(
requestScopePropagator, patchSetId, mergedInto, mergeResultRevId);
}
}

View File

@@ -24,6 +24,7 @@ import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.plugincontext.PluginSetEntryContext;
import com.google.gerrit.server.quota.QuotaResponse.Aggregated;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -100,6 +101,20 @@ public class DefaultQuotaBackend implements QuotaBackend {
return QuotaResponse.Aggregated.create(ImmutableList.copyOf(responses));
}
private static QuotaResponse.Aggregated availableTokens(
PluginSetContext<QuotaEnforcer> quotaEnforcers,
String quotaGroup,
QuotaRequestContext requestContext) {
// PluginSets can change their content when plugins (de-)register. Copy the currently registered
// plugins so that we can iterate twice on a stable list.
List<PluginSetEntryContext<QuotaEnforcer>> enforcers = ImmutableList.copyOf(quotaEnforcers);
List<QuotaResponse> responses = new ArrayList<>(enforcers.size());
for (PluginSetEntryContext<QuotaEnforcer> enforcer : enforcers) {
responses.add(enforcer.call(p -> p.availableTokens(quotaGroup, requestContext)));
}
return QuotaResponse.Aggregated.create(responses);
}
private static void refillAfterErrorOrException(
List<PluginSetEntryContext<QuotaEnforcer>> enforcers,
List<QuotaResponse> collectedResponses,
@@ -158,5 +173,10 @@ public class DefaultQuotaBackend implements QuotaBackend {
return DefaultQuotaBackend.request(
quotaEnforcers, quotaGroup, requestContext, numTokens, false);
}
@Override
public Aggregated availableTokens(String quotaGroup) {
return DefaultQuotaBackend.availableTokens(quotaEnforcers, quotaGroup, requestContext);
}
}
}

View File

@@ -82,5 +82,22 @@ public interface QuotaBackend {
* not to deduct any quota yet. Can be used to do pre-flight requests where necessary
*/
QuotaResponse.Aggregated dryRun(String quotaGroup, long tokens);
/**
* Requests a minimum number of tokens available in implementations. This is a pre-flight check
* for the exceptional case when the requested number of tokens is not known in advance but
* boundary can be specified. For instance, when the commit is received its size is not known
* until the transfer happens however one can specify how many bytes can be accepted to meet the
* repository size quota.
*
* <p>By definition, this is not an allocating request, therefore, it should be followed by the
* call to {@link #requestTokens(String, long)} when the size gets determined so that quota
* could be properly adjusted. It is in developer discretion to ensure that it gets called.
* There might be a case when particular quota gets temporarily overbooked when multiple
* requests are performed but the following calls to {@link #requestTokens(String, long)} will
* fail at the moment when a quota is exhausted. It is not a subject of quota backend to reclaim
* tokens that were used due to overbooking.
*/
QuotaResponse.Aggregated availableTokens(String quotaGroup);
}
}

View File

@@ -59,6 +59,19 @@ public interface QuotaEnforcer {
*/
QuotaResponse dryRun(String quotaGroup, QuotaRequestContext ctx, long numTokens);
/**
* Returns available tokens that can be later requested.
*
* <p>This is used as a pre-flight check for the exceptional case when the requested number of
* tokens is not known in advance. Implementation should not deduct tokens from a bucket. It
* should be followed by a call to {@link #requestTokens(String, QuotaRequestContext, long)} with
* the number of tokens that were eventually used. It is in {@link QuotaBackend} callers
* discretion to ensure that {@link
* com.google.gerrit.server.quota.QuotaBackend.WithResource#requestTokens(String, long)} is
* called.
*/
QuotaResponse availableTokens(String quotaGroup, QuotaRequestContext ctx);
/**
* A previously requested and deducted quota has to be refilled (if possible) because the request
* failed other quota checks. Implementations can choose to leave this a no-op in case they are

View File

@@ -0,0 +1,25 @@
// Copyright (C) 2019 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.google.gerrit.server.quota;
public class QuotaGroupDefinitions {
/**
* Definition of repository size quota group. {@link QuotaEnforcer} implementations for repository
* size quota have to act on requests with this group name.
*/
public static final String REPOSITORY_SIZE_GROUP = "/repository:size";
private QuotaGroupDefinitions() {}
}

View File

@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
import com.google.common.collect.Streams;
import java.util.Collection;
import java.util.Optional;
import java.util.OptionalLong;
import java.util.stream.Collectors;
@AutoValue
@@ -55,6 +56,10 @@ public abstract class QuotaResponse {
return new AutoValue_QuotaResponse.Builder().status(Status.OK).build();
}
public static QuotaResponse ok(long tokens) {
return new AutoValue_QuotaResponse.Builder().status(Status.OK).availableTokens(tokens).build();
}
public static QuotaResponse noOp() {
return new AutoValue_QuotaResponse.Builder().status(Status.NO_OP).build();
}
@@ -65,12 +70,16 @@ public abstract class QuotaResponse {
public abstract Status status();
public abstract Optional<Long> availableTokens();
public abstract Optional<String> message();
@AutoValue.Builder
public abstract static class Builder {
public abstract QuotaResponse.Builder status(Status status);
public abstract QuotaResponse.Builder availableTokens(Long tokens);
public abstract QuotaResponse.Builder message(String message);
public abstract QuotaResponse build();
@@ -96,6 +105,13 @@ public abstract class QuotaResponse {
return responses().stream().filter(r -> r.status().isOk()).collect(toImmutableList());
}
public OptionalLong availableTokens() {
return responses().stream()
.filter(r -> r.status().isOk() && r.availableTokens().isPresent())
.mapToLong(r -> r.availableTokens().get())
.min();
}
public ImmutableList<QuotaResponse> error() {
return responses().stream().filter(r -> r.status().isError()).collect(toImmutableList());
}

View File

@@ -91,7 +91,7 @@ public class LsUserRefs extends SshCommand {
try {
Map<String, Ref> refsMap =
permissionBackend
.user(user)
.user(ctx.getUser())
.project(projectName)
.filter(repo.getRefDatabase().getRefs(), repo, RefFilterOptions.defaults());