Factor out common code from RepoSequence
IntBlob is a simple value class encapsulating the results of reading an
integer blob from a given object ID in a repo. It includes factory
methods for reading/writing blobs to/from the repo, in an interface that
provides everything RepoSequence needs.
In the write case, RepoSequence needs two methods:
* A tryStore method which returns a completed RefUpdate, which may or
may not have succeeded. This return value is essentially an
encapsulation of the RefUpdate.Result and the ref name; the latter
is necessary for producing detailed error messages.
* A checked store method which throws an exception if the result of
tryStore was not success. This in turn delegates to a new
RefUpdateUtil#checkResult method paralleling the existing
checkResults.
Change-Id: Ibbe99972697cb4cb837673f19d7a6fac8b90fce0
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 0000000..edef5cc
--- /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<IntBlob> parse(Repository repo, String refName)
+ throws IOException, OrmException {
+ try (ObjectReader or = repo.newObjectReader()) {
+ return parse(repo, refName, or);
+ }
+ }
+
+ public static Optional<IntBlob> parse(Repository repo, String refName, RevWalk rw)
+ throws IOException, OrmException {
+ return parse(repo, refName, rw.getObjectReader());
+ }
+
+ private static Optional<IntBlob> 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 4c497ac..859b561 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.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 @@
}
@VisibleForTesting
- static RetryerBuilder<RefUpdate.Result> retryerBuilder() {
- return RetryerBuilder.<RefUpdate.Result>newBuilder()
- .retryIfResult(Predicates.equalTo(RefUpdate.Result.LOCK_FAILURE))
+ static RetryerBuilder<RefUpdate> retryerBuilder() {
+ return RetryerBuilder.<RefUpdate>newBuilder()
+ .retryIfResult(ru -> ru != null && RefUpdate.Result.LOCK_FAILURE.equals(ru.getResult()))
.withWaitStrategy(
WaitStrategies.join(
WaitStrategies.exponentialWait(5, TimeUnit.SECONDS),
@@ -85,7 +79,7 @@
.withStopStrategy(StopStrategies.stopAfterDelay(30, TimeUnit.SECONDS));
}
- private static final Retryer<RefUpdate.Result> RETRYER = retryerBuilder().build();
+ private static final Retryer<RefUpdate> RETRYER = retryerBuilder().build();
private final GitRepositoryManager repoManager;
private final GitReferenceUpdated gitRefUpdated;
@@ -95,7 +89,7 @@
private final int floor;
private final int batchSize;
private final Runnable afterReadRef;
- private final Retryer<RefUpdate.Result> retryer;
+ private final Retryer<RefUpdate> retryer;
// Protects all non-final fields.
private final Lock counterLock;
@@ -153,7 +147,7 @@
Seed seed,
int batchSize,
Runnable afterReadRef,
- Retryer<RefUpdate.Result> retryer) {
+ Retryer<RefUpdate> retryer) {
this(repoManager, gitRefUpdated, projectName, name, seed, batchSize, afterReadRef, retryer, 0);
}
@@ -165,7 +159,7 @@
Seed seed,
int batchSize,
Runnable afterReadRef,
- Retryer<RefUpdate.Result> retryer,
+ Retryer<RefUpdate> retryer,
int floor) {
this.repoManager = requireNonNull(repoManager, "repoManager");
this.gitRefUpdated = requireNonNull(gitRefUpdated, "gitRefUpdated");
@@ -234,7 +228,7 @@
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 @@
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 @@
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 @@
}
}
- 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<RefUpdate.Result> {
+ private class TryAcquire implements Callable<RefUpdate> {
private final Repository repo;
private final RevWalk rw;
private final int count;
@@ -307,23 +295,23 @@
}
@Override
- public RefUpdate.Result call() throws Exception {
- Ref ref = repo.exactRef(refName);
+ public RefUpdate call() throws Exception {
+ Optional<IntBlob> 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<RefUpdate.Result> {
+ private class TryIncreaseTo implements Callable<RefUpdate> {
private final Repository repo;
private final RevWalk rw;
private final int value;
@@ -335,60 +323,26 @@
}
@Override
- public RefUpdate.Result call() throws Exception {
- Ref ref = repo.exactRef(refName);
+ public RefUpdate call() throws Exception {
+ Optional<IntBlob> 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 3e33677..514d86a 100644
--- a/java/com/google/gerrit/server/update/RefUpdateUtil.java
+++ b/java/com/google/gerrit/server/update/RefUpdateUtil.java
@@ -105,6 +105,32 @@
}
/**
+ * 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.
*
* <p>Does not require that the ref have any particular old value. Succeeds as a no-op if the ref
diff --git a/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java b/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java
index a21f5ba..64fd875 100644
--- a/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java
+++ b/javatests/com/google/gerrit/server/notedb/RepoSequenceTest.java
@@ -46,7 +46,7 @@
public class RepoSequenceTest {
// Don't sleep in tests.
- private static final Retryer<RefUpdate.Result> RETRYER =
+ private static final Retryer<RefUpdate> RETRYER =
RepoSequence.retryerBuilder().withBlockStrategy(t -> {}).build();
@Rule public ExpectedException exception = ExpectedException.none();
@@ -200,11 +200,11 @@
1,
10,
() -> writeBlob("id", Integer.toString(bgCounter.getAndAdd(1000))),
- RetryerBuilder.<RefUpdate.Result>newBuilder()
+ RetryerBuilder.<RefUpdate>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 @@
1,
10,
() -> writeBlob("id", Integer.toString(bgCounter.getAndAdd(1000))),
- RetryerBuilder.<RefUpdate.Result>newBuilder()
+ RetryerBuilder.<RefUpdate>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 @@
final int start,
int batchSize,
Runnable afterReadRef,
- Retryer<RefUpdate.Result> retryer) {
+ Retryer<RefUpdate> retryer) {
return new RepoSequence(
repoManager,
GitReferenceUpdated.DISABLED,
