diff --git a/java/com/google/gerrit/server/notedb/IntBlob.java b/java/com/google/gerrit/server/notedb/IntBlob.java new file mode 100644 index 0000000000..edef5cc28c --- /dev/null +++ b/java/com/google/gerrit/server/notedb/IntBlob.java @@ -0,0 +1,126 @@ +// Copyright (C) 2018 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.notedb; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; + +import com.google.auto.value.AutoValue; +import com.google.common.base.CharMatcher; +import com.google.common.primitives.Ints; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.extensions.events.GitReferenceUpdated; +import com.google.gerrit.server.update.RefUpdateUtil; +import com.google.gwtorm.server.OrmException; +import java.io.IOException; +import java.util.Optional; +import org.eclipse.jgit.errors.IncorrectObjectTypeException; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.ObjectLoader; +import org.eclipse.jgit.lib.ObjectReader; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.RefUpdate; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevWalk; + +@AutoValue +public abstract class IntBlob { + public static Optional parse(Repository repo, String refName) + throws IOException, OrmException { + try (ObjectReader or = repo.newObjectReader()) { + return parse(repo, refName, or); + } + } + + public static Optional parse(Repository repo, String refName, RevWalk rw) + throws IOException, OrmException { + return parse(repo, refName, rw.getObjectReader()); + } + + private static Optional parse(Repository repo, String refName, ObjectReader or) + throws IOException, OrmException { + Ref ref = repo.exactRef(refName); + if (ref == null) { + return Optional.empty(); + } + ObjectId id = ref.getObjectId(); + ObjectLoader ol = or.open(id, OBJ_BLOB); + if (ol.getType() != OBJ_BLOB) { + // In theory this should be thrown by open but not all implementations may do it properly + // (certainly InMemoryRepository doesn't). + throw new IncorrectObjectTypeException(id, OBJ_BLOB); + } + String str = CharMatcher.whitespace().trimFrom(new String(ol.getCachedBytes(), UTF_8)); + Integer value = Ints.tryParse(str); + if (value == null) { + throw new OrmException("invalid value in " + refName + " blob at " + id.name()); + } + return Optional.of(IntBlob.create(id, value)); + } + + public static RefUpdate tryStore( + Repository repo, + RevWalk rw, + Project.NameKey projectName, + String refName, + @Nullable ObjectId oldId, + int val, + GitReferenceUpdated gitRefUpdated) + throws IOException { + ObjectId newId; + try (ObjectInserter ins = repo.newObjectInserter()) { + newId = ins.insert(OBJ_BLOB, Integer.toString(val).getBytes(UTF_8)); + ins.flush(); + } + RefUpdate ru = repo.updateRef(refName); + if (oldId != null) { + ru.setExpectedOldObjectId(oldId); + } + ru.disableRefLog(); + ru.setNewObjectId(newId); + ru.setForceUpdate(true); // Required for non-commitish updates. + RefUpdate.Result result = ru.update(rw); + if (refUpdated(result)) { + gitRefUpdated.fire(projectName, ru, null); + } + return ru; + } + + public static void store( + Repository repo, + RevWalk rw, + Project.NameKey projectName, + String refName, + @Nullable ObjectId oldId, + int val, + GitReferenceUpdated gitRefUpdated) + throws IOException { + RefUpdateUtil.checkResult(tryStore(repo, rw, projectName, refName, oldId, val, gitRefUpdated)); + } + + private static boolean refUpdated(RefUpdate.Result result) { + return result == RefUpdate.Result.NEW || result == RefUpdate.Result.FORCED; + } + + private static IntBlob create(ObjectId id, int value) { + return new AutoValue_IntBlob(id, value); + } + + public abstract ObjectId id(); + + public abstract int value(); +} diff --git a/java/com/google/gerrit/server/notedb/RepoSequence.java b/java/com/google/gerrit/server/notedb/RepoSequence.java index 4c497ac4a4..859b56163f 100644 --- a/java/com/google/gerrit/server/notedb/RepoSequence.java +++ b/java/com/google/gerrit/server/notedb/RepoSequence.java @@ -27,33 +27,27 @@ import com.github.rholder.retry.RetryerBuilder; import com.github.rholder.retry.StopStrategies; import com.github.rholder.retry.WaitStrategies; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.CharMatcher; -import com.google.common.base.Predicates; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; -import com.google.common.primitives.Ints; import com.google.common.util.concurrent.Runnables; -import com.google.gerrit.common.Nullable; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.update.RefUpdateUtil; import com.google.gwtorm.server.OrmException; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; -import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; -import org.eclipse.jgit.lib.ObjectLoader; -import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; -import org.eclipse.jgit.lib.RefUpdate.Result; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.ReceiveCommand; @@ -75,9 +69,9 @@ public class RepoSequence { } @VisibleForTesting - static RetryerBuilder retryerBuilder() { - return RetryerBuilder.newBuilder() - .retryIfResult(Predicates.equalTo(RefUpdate.Result.LOCK_FAILURE)) + static RetryerBuilder retryerBuilder() { + return RetryerBuilder.newBuilder() + .retryIfResult(ru -> ru != null && RefUpdate.Result.LOCK_FAILURE.equals(ru.getResult())) .withWaitStrategy( WaitStrategies.join( WaitStrategies.exponentialWait(5, TimeUnit.SECONDS), @@ -85,7 +79,7 @@ public class RepoSequence { .withStopStrategy(StopStrategies.stopAfterDelay(30, TimeUnit.SECONDS)); } - private static final Retryer RETRYER = retryerBuilder().build(); + private static final Retryer RETRYER = retryerBuilder().build(); private final GitRepositoryManager repoManager; private final GitReferenceUpdated gitRefUpdated; @@ -95,7 +89,7 @@ public class RepoSequence { private final int floor; private final int batchSize; private final Runnable afterReadRef; - private final Retryer retryer; + private final Retryer retryer; // Protects all non-final fields. private final Lock counterLock; @@ -153,7 +147,7 @@ public class RepoSequence { Seed seed, int batchSize, Runnable afterReadRef, - Retryer retryer) { + Retryer retryer) { this(repoManager, gitRefUpdated, projectName, name, seed, batchSize, afterReadRef, retryer, 0); } @@ -165,7 +159,7 @@ public class RepoSequence { Seed seed, int batchSize, Runnable afterReadRef, - Retryer retryer, + Retryer retryer, int floor) { this.repoManager = requireNonNull(repoManager, "repoManager"); this.gitRefUpdated = requireNonNull(gitRefUpdated, "gitRefUpdated"); @@ -234,7 +228,7 @@ public class RepoSequence { try { try (Repository repo = repoManager.openRepository(projectName); RevWalk rw = new RevWalk(repo)) { - checkResult(store(repo, rw, null, val)); + IntBlob.store(repo, rw, projectName, refName, null, val, gitRefUpdated); counter = limit; } catch (IOException e) { throw new OrmException(e); @@ -250,7 +244,11 @@ public class RepoSequence { try (Repository repo = repoManager.openRepository(projectName); RevWalk rw = new RevWalk(repo)) { TryIncreaseTo attempt = new TryIncreaseTo(repo, rw, val); - checkResult(retryer.call(attempt)); + RefUpdate ru = retryer.call(attempt); + // Null update is a sentinel meaning nothing to do. + if (ru != null) { + RefUpdateUtil.checkResult(ru); + } counter = limit; } catch (ExecutionException | RetryException e) { if (e.getCause() != null) { @@ -269,7 +267,7 @@ public class RepoSequence { try (Repository repo = repoManager.openRepository(projectName); RevWalk rw = new RevWalk(repo)) { TryAcquire attempt = new TryAcquire(repo, rw, count); - checkResult(retryer.call(attempt)); + RefUpdateUtil.checkResult(retryer.call(attempt)); counter = attempt.next; limit = counter + count; acquireCount++; @@ -283,17 +281,7 @@ public class RepoSequence { } } - private void checkResult(RefUpdate.Result result) throws OrmException { - if (!refUpdated(result) && result != Result.NO_CHANGE) { - throw new OrmException("failed to update " + refName + ": " + result); - } - } - - private boolean refUpdated(RefUpdate.Result result) { - return result == RefUpdate.Result.NEW || result == RefUpdate.Result.FORCED; - } - - private class TryAcquire implements Callable { + private class TryAcquire implements Callable { private final Repository repo; private final RevWalk rw; private final int count; @@ -307,23 +295,23 @@ public class RepoSequence { } @Override - public RefUpdate.Result call() throws Exception { - Ref ref = repo.exactRef(refName); + public RefUpdate call() throws Exception { + Optional blob = IntBlob.parse(repo, refName, rw); afterReadRef.run(); ObjectId oldId; - if (ref == null) { + if (!blob.isPresent()) { oldId = ObjectId.zeroId(); next = seed.get(); } else { - oldId = ref.getObjectId(); - next = parse(rw, oldId); + oldId = blob.get().id(); + next = blob.get().value(); } next = Math.max(floor, next); - return store(repo, rw, oldId, next + count); + return IntBlob.tryStore(repo, rw, projectName, refName, oldId, next + count, gitRefUpdated); } } - private class TryIncreaseTo implements Callable { + private class TryIncreaseTo implements Callable { private final Repository repo; private final RevWalk rw; private final int value; @@ -335,60 +323,26 @@ public class RepoSequence { } @Override - public RefUpdate.Result call() throws Exception { - Ref ref = repo.exactRef(refName); + public RefUpdate call() throws Exception { + Optional blob = IntBlob.parse(repo, refName, rw); afterReadRef.run(); ObjectId oldId; - if (ref == null) { + if (!blob.isPresent()) { oldId = ObjectId.zeroId(); } else { - oldId = ref.getObjectId(); - int next = parse(rw, oldId); + oldId = blob.get().id(); + int next = blob.get().value(); if (next >= value) { - // a concurrent write updated the ref already to this or a higher value - return RefUpdate.Result.NO_CHANGE; + // A concurrent write updated the ref already to this or a higher value; return null as a + // sentinel meaning nothing to do. Returning RefUpdate doesn't give us the flexibility to + // return any other kind of sentinel, since it's a fairly thick object. + return null; } } - return store(repo, rw, oldId, value); + return IntBlob.tryStore(repo, rw, projectName, refName, oldId, value, gitRefUpdated); } } - private int parse(RevWalk rw, ObjectId id) throws IOException, OrmException { - ObjectLoader ol = rw.getObjectReader().open(id, OBJ_BLOB); - if (ol.getType() != OBJ_BLOB) { - // In theory this should be thrown by open but not all implementations - // may do it properly (certainly InMemoryRepository doesn't). - throw new IncorrectObjectTypeException(id, OBJ_BLOB); - } - String str = CharMatcher.whitespace().trimFrom(new String(ol.getCachedBytes(), UTF_8)); - Integer val = Ints.tryParse(str); - if (val == null) { - throw new OrmException("invalid value in " + refName + " blob at " + id.name()); - } - return val; - } - - private RefUpdate.Result store(Repository repo, RevWalk rw, @Nullable ObjectId oldId, int val) - throws IOException { - ObjectId newId; - try (ObjectInserter ins = repo.newObjectInserter()) { - newId = ins.insert(OBJ_BLOB, Integer.toString(val).getBytes(UTF_8)); - ins.flush(); - } - RefUpdate ru = repo.updateRef(refName); - if (oldId != null) { - ru.setExpectedOldObjectId(oldId); - } - ru.disableRefLog(); - ru.setNewObjectId(newId); - ru.setForceUpdate(true); // Required for non-commitish updates. - RefUpdate.Result result = ru.update(rw); - if (refUpdated(result)) { - gitRefUpdated.fire(projectName, ru, null); - } - return result; - } - public static ReceiveCommand storeNew(ObjectInserter ins, String name, int val) throws IOException { ObjectId newId = ins.insert(OBJ_BLOB, Integer.toString(val).getBytes(UTF_8)); diff --git a/java/com/google/gerrit/server/update/RefUpdateUtil.java b/java/com/google/gerrit/server/update/RefUpdateUtil.java index 3e336776bf..514d86a435 100644 --- a/java/com/google/gerrit/server/update/RefUpdateUtil.java +++ b/java/com/google/gerrit/server/update/RefUpdateUtil.java @@ -104,6 +104,32 @@ public class RefUpdateUtil { } } + /** + * Check results of a single ref update, throwing an exception if there was a failure. + * + * @param ru ref update; must already have been executed. + * @throws IllegalArgumentException if the result was {@code NOT_ATTEMPTED}. + * @throws LockFailureException if the result was {@code LOCK_FAILURE}. + * @throws IOException if the result failed for another reason. + */ + public static void checkResult(RefUpdate ru) throws IOException { + RefUpdate.Result result = ru.getResult(); + switch (result) { + case NOT_ATTEMPTED: + throw new IllegalArgumentException("Not attempted: " + ru.getName()); + case NEW: + case FORCED: + case NO_CHANGE: + case FAST_FORWARD: + case RENAMED: + return; + case LOCK_FAILURE: + throw new LockFailureException("Failed to update " + ru.getName() + ": " + result, ru); + default: + throw new IOException("Failed to update " + ru.getName() + ": " + ru.getResult()); + } + } + /** * Delete a single ref, throwing a checked exception on failure. * diff --git a/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java b/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java index a21f5bae0c..64fd8757cf 100644 --- a/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java +++ b/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java @@ -46,7 +46,7 @@ import org.junit.rules.ExpectedException; public class RepoSequenceTest { // Don't sleep in tests. - private static final Retryer RETRYER = + private static final Retryer RETRYER = RepoSequence.retryerBuilder().withBlockStrategy(t -> {}).build(); @Rule public ExpectedException exception = ExpectedException.none(); @@ -200,11 +200,11 @@ public class RepoSequenceTest { 1, 10, () -> writeBlob("id", Integer.toString(bgCounter.getAndAdd(1000))), - RetryerBuilder.newBuilder() + RetryerBuilder.newBuilder() .withStopStrategy(StopStrategies.stopAfterAttempt(3)) .build()); exception.expect(OrmException.class); - exception.expectMessage("failed to update refs/sequences/id: LOCK_FAILURE"); + exception.expectMessage("Failed to update refs/sequences/id: LOCK_FAILURE"); s.next(); } @@ -335,11 +335,11 @@ public class RepoSequenceTest { 1, 10, () -> writeBlob("id", Integer.toString(bgCounter.getAndAdd(1000))), - RetryerBuilder.newBuilder() + RetryerBuilder.newBuilder() .withStopStrategy(StopStrategies.stopAfterAttempt(3)) .build()); exception.expect(OrmException.class); - exception.expectMessage("failed to update refs/sequences/id: LOCK_FAILURE"); + exception.expectMessage("Failed to update refs/sequences/id: LOCK_FAILURE"); s.increaseTo(2); } @@ -352,7 +352,7 @@ public class RepoSequenceTest { final int start, int batchSize, Runnable afterReadRef, - Retryer retryer) { + Retryer retryer) { return new RepoSequence( repoManager, GitReferenceUpdated.DISABLED,