Merge changes from topic "notedb-pack-inserter" into stable-2.15
* changes:
NoteDbMigrator: Use new pack-based inserter implementation
Upgrade JGit to 4.9.0.201710071750-r.8-g678c99c05
NoteDbMigrator: Use one NoteDbUpdateManager per project
LockFailureException: Expose ref names
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
index 51e2964..6b14263 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
@@ -60,8 +60,17 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.List;
+import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
@@ -374,10 +383,14 @@
Change.Id id1 = r1.getChange().getId();
Change.Id id2 = r2.getChange().getId();
- migrate(b -> b.setThreads(threads));
- assertNotesMigrationState(NOTE_DB, false, false);
+ Set<String> objectFiles = getObjectFiles(project);
+ assertThat(objectFiles).isNotEmpty();
+ migrate(b -> b.setThreads(threads));
+
+ assertNotesMigrationState(NOTE_DB, false, false);
assertThat(sequences.nextChangeId()).isEqualTo(503);
+ assertThat(getObjectFiles(project)).containsExactlyElementsIn(objectFiles);
ObjectId oldMetaId = null;
int rowVersion = 0;
@@ -531,4 +544,23 @@
private void addListener(NotesMigrationStateListener listener) {
addedListeners.add(listeners.add(listener));
}
+
+ private SortedSet<String> getObjectFiles(Project.NameKey project) throws Exception {
+ SortedSet<String> files = new TreeSet<>();
+ try (Repository repo = repoManager.openRepository(project)) {
+ Files.walkFileTree(
+ ((FileRepository) repo).getObjectDatabase().getDirectory().toPath(),
+ new SimpleFileVisitor<Path>() {
+ @Override
+ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
+ String name = file.getFileName().toString();
+ if (!attrs.isDirectory() && !name.endsWith(".pack") && !name.endsWith(".idx")) {
+ files.add(name);
+ }
+ return FileVisitResult.CONTINUE;
+ }
+ });
+ }
+ return files;
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
index 00dc05a5..db37147 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/externalids/ExternalIdsUpdate.java
@@ -840,7 +840,7 @@
case FORCED:
break;
case LOCK_FAILURE:
- throw new LockFailureException("Updating external IDs failed with " + res);
+ throw new LockFailureException("Updating external IDs failed with " + res, u);
case IO_FAILURE:
case NOT_ATTEMPTED:
case REJECTED:
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java
index 9c43bdb..b80f846 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/InMemoryInserter.java
@@ -95,6 +95,10 @@
return ImmutableList.copyOf(inserted.values());
}
+ public int getInsertedObjectCount() {
+ return inserted.values().size();
+ }
+
public void clear() {
inserted.clear();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LockFailureException.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LockFailureException.java
index 7380b0a..02a30e0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/LockFailureException.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LockFailureException.java
@@ -14,13 +14,38 @@
package com.google.gerrit.server.git;
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.common.collect.ImmutableList;
import java.io.IOException;
+import org.eclipse.jgit.lib.BatchRefUpdate;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.transport.ReceiveCommand;
/** Thrown when updating a ref in Git fails with LOCK_FAILURE. */
public class LockFailureException extends IOException {
private static final long serialVersionUID = 1L;
- public LockFailureException(String message) {
+ private final ImmutableList<String> refs;
+
+ public LockFailureException(String message, RefUpdate refUpdate) {
super(message);
+ refs = ImmutableList.of(refUpdate.getName());
+ }
+
+ public LockFailureException(String message, BatchRefUpdate batchRefUpdate) {
+ super(message);
+ refs =
+ batchRefUpdate
+ .getCommands()
+ .stream()
+ .filter(c -> c.getResult() == ReceiveCommand.Result.LOCK_FAILURE)
+ .map(ReceiveCommand::getRefName)
+ .collect(toImmutableList());
+ }
+
+ /** Subset of ref names that caused the lock failure. */
+ public ImmutableList<String> getFailedRefs() {
+ return refs;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
index 1bf3586..92edc47 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
@@ -395,7 +395,8 @@
+ " in "
+ db.getDirectory()
+ ": "
- + ru.getResult());
+ + ru.getResult(),
+ ru);
case FORCED:
case IO_FAILURE:
case NOT_ATTEMPTED:
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
index c9a28f4..7799d2d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java
@@ -779,6 +779,7 @@
rebuildResult = checkNotNull(r);
checkNotNull(r.newState());
checkNotNull(r.staged());
+ checkNotNull(r.staged().changeObjects());
return LoadHandle.create(
ChangeNotesCommit.newStagedRevWalk(repo, r.staged().changeObjects()),
r.newState().getChangeMetaId());
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index ea1f891..3e36de9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -98,13 +98,13 @@
ImmutableList<InsertedObject> changeObjects = ImmutableList.of();
if (changeRepo != null) {
changeCommands = changeRepo.getCommandsSnapshot();
- changeObjects = changeRepo.tempIns.getInsertedObjects();
+ changeObjects = changeRepo.getInsertedObjects();
}
ImmutableList<ReceiveCommand> allUsersCommands = ImmutableList.of();
ImmutableList<InsertedObject> allUsersObjects = ImmutableList.of();
if (allUsersRepo != null) {
allUsersCommands = allUsersRepo.getCommandsSnapshot();
- allUsersObjects = allUsersRepo.tempIns.getInsertedObjects();
+ allUsersObjects = allUsersRepo.getInsertedObjects();
}
return new AutoValue_NoteDbUpdateManager_StagedResult(
id, delta,
@@ -119,10 +119,32 @@
public abstract ImmutableList<ReceiveCommand> changeCommands();
+ /**
+ * Objects inserted into the change repo for this change.
+ *
+ * <p>Includes all objects inserted for any change in this repo that may have been processed by
+ * the corresponding {@link NoteDbUpdateManager} instance, not just those objects that were
+ * inserted to handle this specific change's updates.
+ *
+ * @return inserted objects, or null if the corresponding {@link NoteDbUpdateManager} was
+ * configured not to {@link NoteDbUpdateManager#setSaveObjects(boolean) save objects}.
+ */
+ @Nullable
public abstract ImmutableList<InsertedObject> changeObjects();
public abstract ImmutableList<ReceiveCommand> allUsersCommands();
+ /**
+ * Objects inserted into the All-Users repo for this change.
+ *
+ * <p>Includes all objects inserted into All-Users for any change that may have been processed
+ * by the corresponding {@link NoteDbUpdateManager} instance, not just those objects that were
+ * inserted to handle this specific change's updates.
+ *
+ * @return inserted objects, or null if the corresponding {@link NoteDbUpdateManager} was
+ * configured not to {@link NoteDbUpdateManager#setSaveObjects(boolean) save objects}.
+ */
+ @Nullable
public abstract ImmutableList<InsertedObject> allUsersObjects();
}
@@ -144,17 +166,20 @@
public final RevWalk rw;
public final ChainedReceiveCommands cmds;
- private final InMemoryInserter tempIns;
+ private final InMemoryInserter inMemIns;
+ private final ObjectInserter tempIns;
@Nullable private final ObjectInserter finalIns;
private final boolean close;
+ private final boolean saveObjects;
private OpenRepo(
Repository repo,
RevWalk rw,
@Nullable ObjectInserter ins,
ChainedReceiveCommands cmds,
- boolean close) {
+ boolean close,
+ boolean saveObjects) {
ObjectReader reader = rw.getObjectReader();
checkArgument(
ins == null || reader.getCreatedFromInserter() == ins,
@@ -162,11 +187,21 @@
ins,
reader.getCreatedFromInserter());
this.repo = checkNotNull(repo);
- this.tempIns = new InMemoryInserter(rw.getObjectReader());
+
+ if (saveObjects) {
+ this.inMemIns = new InMemoryInserter(rw.getObjectReader());
+ this.tempIns = inMemIns;
+ } else {
+ checkArgument(ins != null);
+ this.inMemIns = null;
+ this.tempIns = ins;
+ }
+
this.rw = new RevWalk(tempIns.newReader());
this.finalIns = ins;
this.cmds = checkNotNull(cmds);
this.close = close;
+ this.saveObjects = saveObjects;
}
public Optional<ObjectId> getObjectId(String refName) throws IOException {
@@ -177,17 +212,25 @@
return ImmutableList.copyOf(cmds.getCommands().values());
}
+ @Nullable
+ ImmutableList<InsertedObject> getInsertedObjects() {
+ return saveObjects ? inMemIns.getInsertedObjects() : null;
+ }
+
void flush() throws IOException {
flushToFinalInserter();
finalIns.flush();
}
void flushToFinalInserter() throws IOException {
+ if (!saveObjects) {
+ return;
+ }
checkState(finalIns != null);
- for (InsertedObject obj : tempIns.getInsertedObjects()) {
+ for (InsertedObject obj : inMemIns.getInsertedObjects()) {
finalIns.insert(obj.type(), obj.data().toByteArray());
}
- tempIns.clear();
+ inMemIns.clear();
}
@Override
@@ -219,6 +262,8 @@
private OpenRepo allUsersRepo;
private Map<Change.Id, StagedResult> staged;
private boolean checkExpectedState = true;
+ private boolean saveObjects = true;
+ private boolean atomicRefUpdates = true;
private String refLogMessage;
private PersonIdent refLogIdent;
private PushCertificate pushCert;
@@ -264,14 +309,14 @@
public NoteDbUpdateManager setChangeRepo(
Repository repo, RevWalk rw, @Nullable ObjectInserter ins, ChainedReceiveCommands cmds) {
checkState(changeRepo == null, "change repo already initialized");
- changeRepo = new OpenRepo(repo, rw, ins, cmds, false);
+ changeRepo = new OpenRepo(repo, rw, ins, cmds, false, saveObjects);
return this;
}
public NoteDbUpdateManager setAllUsersRepo(
Repository repo, RevWalk rw, @Nullable ObjectInserter ins, ChainedReceiveCommands cmds) {
checkState(allUsersRepo == null, "All-Users repo already initialized");
- allUsersRepo = new OpenRepo(repo, rw, ins, cmds, false);
+ allUsersRepo = new OpenRepo(repo, rw, ins, cmds, false, saveObjects);
return this;
}
@@ -280,6 +325,37 @@
return this;
}
+ /**
+ * Set whether to save objects and make them available in {@link StagedResult}s.
+ *
+ * <p>If set, all objects inserted into all repos managed by this instance will be buffered in
+ * memory, and the {@link StagedResult}s will return non-null lists from {@link
+ * StagedResult#changeObjects()} and {@link StagedResult#allUsersObjects()}.
+ *
+ * <p>Not recommended if modifying a large number of changes with a single manager.
+ *
+ * @param saveObjects whether to save objects; defaults to true.
+ * @return this
+ */
+ public NoteDbUpdateManager setSaveObjects(boolean saveObjects) {
+ this.saveObjects = saveObjects;
+ return this;
+ }
+
+ /**
+ * Set whether to use atomic ref updates.
+ *
+ * <p>Can be set to false when the change updates represented by this manager aren't logically
+ * related, e.g. when the updater is only used to group objects together with a single inserter.
+ *
+ * @param atomicRefUpdates whether to use atomic ref updates; defaults to true.
+ * @return this
+ */
+ public NoteDbUpdateManager setAtomicRefUpdates(boolean atomicRefUpdates) {
+ this.atomicRefUpdates = atomicRefUpdates;
+ return this;
+ }
+
public NoteDbUpdateManager setRefLogMessage(String message) {
this.refLogMessage = message;
return this;
@@ -336,7 +412,7 @@
ObjectInserter ins = repo.newObjectInserter(); // Closed by OpenRepo#close.
ObjectReader reader = ins.newReader(); // Not closed by OpenRepo#close.
try (RevWalk rw = new RevWalk(reader)) { // Doesn't escape OpenRepo constructor.
- return new OpenRepo(repo, rw, ins, new ChainedReceiveCommands(repo), true) {
+ return new OpenRepo(repo, rw, ins, new ChainedReceiveCommands(repo), true, saveObjects) {
@Override
public void close() {
reader.close();
@@ -543,6 +619,7 @@
} else {
// OpenRepo buffers objects separately; caller may assume that objects are available in the
// inserter it previously passed via setChangeRepo.
+ checkState(saveObjects, "cannot use dryrun with saveObjects = false");
or.flushToFinalInserter();
}
@@ -554,6 +631,7 @@
bru.setRefLogMessage(firstNonNull(guessRestApiHandler(), "Update NoteDb refs"), false);
}
bru.setRefLogIdent(refLogIdent != null ? refLogIdent : serverIdent.get());
+ bru.setAtomic(atomicRefUpdates);
or.cmds.addTo(bru);
bru.setAllowNonFastForwards(true);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
index 576a8dd..f96e96c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/ChangeRebuilderImpl.java
@@ -187,7 +187,7 @@
}
try (NoteDbUpdateManager manager = updateManagerFactory.create(change.getProject())) {
buildUpdates(manager, bundleReader.fromReviewDb(db, changeId));
- return execute(db, changeId, manager, checkReadOnly);
+ return execute(db, changeId, manager, checkReadOnly, true);
}
}
@@ -216,11 +216,15 @@
@Override
public Result execute(ReviewDb db, Change.Id changeId, NoteDbUpdateManager manager)
throws OrmException, IOException {
- return execute(db, changeId, manager, true);
+ return execute(db, changeId, manager, true, true);
}
public Result execute(
- ReviewDb db, Change.Id changeId, NoteDbUpdateManager manager, boolean checkReadOnly)
+ ReviewDb db,
+ Change.Id changeId,
+ NoteDbUpdateManager manager,
+ boolean checkReadOnly,
+ boolean executeManager)
throws OrmException, IOException {
db = ReviewDbUtil.unwrapDb(db);
Change change = checkNoteDbState(ChangeNotes.readOneReviewDbChange(db, changeId));
@@ -277,11 +281,13 @@
// to the caller so they know to use the staged results instead of reading from the repo.
throw new OrmException(NoteDbUpdateManager.CHANGES_READ_ONLY);
}
- manager.execute();
+ if (executeManager) {
+ manager.execute();
+ }
return r;
}
- private static Change checkNoteDbState(Change c) throws OrmException {
+ static Change checkNoteDbState(Change c) throws OrmException {
// Can only rebuild a change if its primary storage is ReviewDb.
NoteDbChangeState s = NoteDbChangeState.parse(c);
if (s != null && s.getPrimaryStorage() != PrimaryStorage.REVIEW_DB) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
index add6d57..2981174 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
@@ -26,6 +26,7 @@
import static com.google.gerrit.server.notedb.NotesMigrationState.WRITE;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Comparator.comparing;
+import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
import com.google.common.annotations.VisibleForTesting;
@@ -56,12 +57,17 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LockFailureException;
import com.google.gerrit.server.git.WorkQueue;
+import com.google.gerrit.server.notedb.ChangeBundleReader;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.MutableNotesMigration;
import com.google.gerrit.server.notedb.NoteDbTable;
+import com.google.gerrit.server.notedb.NoteDbUpdateManager;
import com.google.gerrit.server.notedb.NotesMigrationState;
import com.google.gerrit.server.notedb.PrimaryStorageMigrator;
import com.google.gerrit.server.notedb.RepoSequence;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException;
+import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.update.ChainedReceiveCommands;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gwtorm.server.OrmException;
@@ -74,17 +80,24 @@
import java.io.PrintWriter;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashSet;
import java.util.List;
import java.util.Optional;
+import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import java.util.function.Predicate;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.ProgressMonitor;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.TextProgressMonitor;
+import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
import org.eclipse.jgit.util.io.NullOutputStream;
@@ -119,10 +132,12 @@
private final SitePaths sitePaths;
private final SchemaFactory<ReviewDb> schemaFactory;
private final GitRepositoryManager repoManager;
+ private final NoteDbUpdateManager.Factory updateManagerFactory;
+ private final ChangeBundleReader bundleReader;
private final AllProjectsName allProjects;
private final InternalUser.Factory userFactory;
private final ThreadLocalRequestContext requestContext;
- private final ChangeRebuilder rebuilder;
+ private final ChangeRebuilderImpl rebuilder;
private final WorkQueue workQueue;
private final MutableNotesMigration globalNotesMigration;
private final PrimaryStorageMigrator primaryStorageMigrator;
@@ -144,10 +159,12 @@
SitePaths sitePaths,
SchemaFactory<ReviewDb> schemaFactory,
GitRepositoryManager repoManager,
+ NoteDbUpdateManager.Factory updateManagerFactory,
+ ChangeBundleReader bundleReader,
AllProjectsName allProjects,
ThreadLocalRequestContext requestContext,
InternalUser.Factory userFactory,
- ChangeRebuilder rebuilder,
+ ChangeRebuilderImpl rebuilder,
WorkQueue workQueue,
MutableNotesMigration globalNotesMigration,
PrimaryStorageMigrator primaryStorageMigrator,
@@ -159,6 +176,8 @@
this.sitePaths = sitePaths;
this.schemaFactory = schemaFactory;
this.repoManager = repoManager;
+ this.updateManagerFactory = updateManagerFactory;
+ this.bundleReader = bundleReader;
this.allProjects = allProjects;
this.requestContext = requestContext;
this.userFactory = userFactory;
@@ -318,6 +337,8 @@
sitePaths,
schemaFactory,
repoManager,
+ updateManagerFactory,
+ bundleReader,
allProjects,
requestContext,
userFactory,
@@ -343,10 +364,12 @@
private final FileBasedConfig noteDbConfig;
private final SchemaFactory<ReviewDb> schemaFactory;
private final GitRepositoryManager repoManager;
+ private final NoteDbUpdateManager.Factory updateManagerFactory;
+ private final ChangeBundleReader bundleReader;
private final AllProjectsName allProjects;
private final ThreadLocalRequestContext requestContext;
private final InternalUser.Factory userFactory;
- private final ChangeRebuilder rebuilder;
+ private final ChangeRebuilderImpl rebuilder;
private final MutableNotesMigration globalNotesMigration;
private final PrimaryStorageMigrator primaryStorageMigrator;
private final DynamicSet<NotesMigrationStateListener> listeners;
@@ -365,10 +388,12 @@
SitePaths sitePaths,
SchemaFactory<ReviewDb> schemaFactory,
GitRepositoryManager repoManager,
+ NoteDbUpdateManager.Factory updateManagerFactory,
+ ChangeBundleReader bundleReader,
AllProjectsName allProjects,
ThreadLocalRequestContext requestContext,
InternalUser.Factory userFactory,
- ChangeRebuilder rebuilder,
+ ChangeRebuilderImpl rebuilder,
MutableNotesMigration globalNotesMigration,
PrimaryStorageMigrator primaryStorageMigrator,
DynamicSet<NotesMigrationStateListener> listeners,
@@ -392,6 +417,8 @@
this.schemaFactory = schemaFactory;
this.rebuilder = rebuilder;
this.repoManager = repoManager;
+ this.updateManagerFactory = updateManagerFactory;
+ this.bundleReader = bundleReader;
this.allProjects = allProjects;
this.requestContext = requestContext;
this.userFactory = userFactory;
@@ -720,6 +747,12 @@
return ImmutableListMultimap.copyOf(out);
}
+ private static ObjectInserter newPackInserter(Repository repo) {
+ return repo instanceof FileRepository
+ ? ((FileRepository) repo).getObjectDatabase().newPackInserter()
+ : repo.newObjectInserter();
+ }
+
private boolean rebuildProject(
ReviewDb db,
ImmutableListMultimap<Project.NameKey, Change.Id> allChanges,
@@ -729,43 +762,105 @@
ProgressMonitor pm =
new TextProgressMonitor(
new PrintWriter(new BufferedWriter(new OutputStreamWriter(progressOut, UTF_8))));
- pm.beginTask(FormatUtil.elide(project.get(), 50), allChanges.get(project).size());
- try {
+ try (Repository changeRepo = repoManager.openRepository(project);
+ ObjectInserter changeIns = newPackInserter(changeRepo);
+ ObjectReader changeReader = changeIns.newReader();
+ RevWalk changeRw = new RevWalk(changeReader);
+ NoteDbUpdateManager manager =
+ updateManagerFactory
+ .create(project)
+ .setSaveObjects(false)
+ .setAtomicRefUpdates(false)
+ // Only use a PackInserter for the change repo, not All-Users.
+ //
+ // It's not possible to share a single inserter for All-Users across all project
+ // tasks, and we don't want to add one pack per project to All-Users. Adding many
+ // loose objects is preferable to many packs.
+ //
+ // Anyway, the number of objects inserted into All-Users is proportional to the
+ // number of pending draft comments, which should not be high (relative to the total
+ // number of changes), so the number of loose objects shouldn't be too unreasonable.
+ .setChangeRepo(
+ changeRepo, changeRw, changeIns, new ChainedReceiveCommands(changeRepo))) {
+ Set<Change.Id> skipExecute = new HashSet<>();
Collection<Change.Id> changes = allChanges.get(project);
- for (Change.Id changeId : changes) {
- // Update one change at a time, which ends up creating one NoteDbUpdateManager per change as
- // well. This turns out to be no more expensive than batching, since each NoteDb operation
- // is only writing single loose ref updates and loose objects. Plus we have to do one
- // ReviewDb transaction per change due to the AtomicUpdate, so if we somehow batched NoteDb
- // operations, ReviewDb would become the bottleneck.
- try {
- rebuilder.rebuild(db, changeId);
- } catch (NoPatchSetsException e) {
- log.warn(e.getMessage());
- } catch (RepositoryNotFoundException e) {
- log.warn("Repository {} not found while rebuilding change {}", project, changeId);
- } catch (ConflictingUpdateException e) {
- log.warn(
- "Rebuilding detected a conflicting ReviewDb update for change {};"
- + " will be auto-rebuilt at runtime",
- changeId);
- } catch (LockFailureException e) {
- log.warn(
- "Rebuilding detected a conflicting NoteDb update for change {};"
- + " will be auto-rebuilt at runtime",
- changeId);
- } catch (Throwable t) {
- log.error("Failed to rebuild change " + changeId, t);
- ok = false;
+ pm.beginTask(FormatUtil.elide("Rebuilding " + project.get(), 50), changes.size());
+ try {
+ for (Change.Id changeId : changes) {
+ boolean staged = false;
+ try {
+ stage(db, changeId, manager);
+ staged = true;
+ } catch (NoPatchSetsException e) {
+ log.warn(e.getMessage());
+ } catch (Throwable t) {
+ log.error("Failed to rebuild change " + changeId, t);
+ ok = false;
+ }
+ pm.update(1);
+ if (!staged) {
+ skipExecute.add(changeId);
+ }
}
- pm.update(1);
+ } finally {
+ pm.endTask();
}
- } finally {
- pm.endTask();
+
+ pm.beginTask(
+ FormatUtil.elide("Saving " + project.get(), 50), changes.size() - skipExecute.size());
+ try {
+ for (Change.Id changeId : changes) {
+ if (skipExecute.contains(changeId)) {
+ continue;
+ }
+ try {
+ rebuilder.execute(db, changeId, manager, true, false);
+ } catch (ConflictingUpdateException e) {
+ log.warn(
+ "Rebuilding detected a conflicting ReviewDb update for change {};"
+ + " will be auto-rebuilt at runtime",
+ changeId);
+ } catch (Throwable t) {
+ log.error("Failed to rebuild change " + changeId, t);
+ ok = false;
+ }
+ pm.update(1);
+ }
+ } finally {
+ pm.endTask();
+ }
+
+ try {
+ manager.execute();
+ } catch (LockFailureException e) {
+ log.warn(
+ "Rebuilding detected a conflicting NoteDb update for the following refs, which will"
+ + " be auto-rebuilt at runtime: {}",
+ e.getFailedRefs().stream().distinct().sorted().collect(joining(", ")));
+ } catch (OrmException | IOException e) {
+ log.error("Failed to save NoteDb state for " + project, e);
+ }
+ } catch (RepositoryNotFoundException e) {
+ log.warn("Repository {} not found", project);
+ } catch (IOException e) {
+ log.error("Failed to rebuild project " + project, e);
}
return ok;
}
+ private void stage(ReviewDb db, Change.Id changeId, NoteDbUpdateManager manager)
+ throws OrmException, IOException {
+ // Match ChangeRebuilderImpl#stage, but without calling manager.stage(), since that can only be
+ // called after building updates for all changes.
+ Change change =
+ ChangeRebuilderImpl.checkNoteDbState(ChangeNotes.readOneReviewDbChange(db, changeId));
+ if (change == null) {
+ // Could log here instead, but this matches the behavior of ChangeRebuilderImpl#stage.
+ throw new NoSuchChangeException(changeId);
+ }
+ rebuilder.buildUpdates(manager, bundleReader.fromReviewDb(db, changeId));
+ }
+
private static boolean futuresToBoolean(List<ListenableFuture<Boolean>> futures, String errMsg) {
try {
return Futures.allAsList(futures).get().stream().allMatch(b -> b);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/update/RefUpdateUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/update/RefUpdateUtil.java
index ab0b78e..86b4eef 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/update/RefUpdateUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/update/RefUpdateUtil.java
@@ -74,7 +74,7 @@
}
if (lockFailure + aborted == bru.getCommands().size()) {
- throw new LockFailureException("Update aborted with one or more lock failures: " + bru);
+ throw new LockFailureException("Update aborted with one or more lock failures: " + bru, bru);
} else if (failure > 0) {
throw new IOException("Update failed: " + bru);
}
diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl
index 1d964b8..27f53c7 100644
--- a/lib/jgit/jgit.bzl
+++ b/lib/jgit/jgit.bzl
@@ -1,12 +1,12 @@
load("//tools/bzl:maven_jar.bzl", "GERRIT", "MAVEN_LOCAL", "MAVEN_CENTRAL", "maven_jar")
-_JGIT_VERS = "4.9.0.201710071750-r"
+_JGIT_VERS = "4.9.0.201710071750-r.8-g678c99c05"
-_DOC_VERS = _JGIT_VERS # Set to _JGIT_VERS unless using a snapshot
+_DOC_VERS = "4.9.0.201710071750-r" # Set to _JGIT_VERS unless using a snapshot
JGIT_DOC_URL = "http://download.eclipse.org/jgit/site/" + _DOC_VERS + "/apidocs"
-_JGIT_REPO = MAVEN_CENTRAL # Leave here even if set to MAVEN_CENTRAL.
+_JGIT_REPO = GERRIT # Leave here even if set to MAVEN_CENTRAL.
# set this to use a local version.
# "/home/<user>/projects/jgit"
@@ -26,28 +26,28 @@
name = "jgit_lib",
artifact = "org.eclipse.jgit:org.eclipse.jgit:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "69d8510b335d4d33d551a133505a4141311f970a",
- src_sha1 = "6fd1eb331447b6163898b4d10aa769e2ac193740",
+ sha1 = "845fcaabaaf84f0924f78971def3adc962d3a69e",
+ src_sha1 = "8cc0627939d1bcd8fb313fd6bed03edfecc5011f",
unsign = True,
)
maven_jar(
name = "jgit_servlet",
artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "93fb0075988b9c6bb97c725c03706f2341965b6b",
+ sha1 = "a0cb65ef0d9db387b010337b44ac4710be52b21e",
unsign = True,
)
maven_jar(
name = "jgit_archive",
artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "a15aee805c758516ad7e9fa3f16e27bb9f4a1c2e",
+ sha1 = "32c85ea349d19a0129d43ceda021a3bdd41a8242",
)
maven_jar(
name = "jgit_junit",
artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + _JGIT_VERS,
repository = _JGIT_REPO,
- sha1 = "b6e712e743ea5798134f54547ae80456fad07f76",
+ sha1 = "549c3b338d66c2f8575394252a39e69d7fe71969",
unsign = True,
)
