From ecd4831c732a656d68830d51fc9448af6f7e8e45 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Mon, 24 Apr 2017 11:36:52 -0400 Subject: [PATCH 1/2] TestNotesMigration: Initialize from env during constructor I want to use TestNotesMigration from within InMemoryRepository at the time the schema is created. The previous approach of calling setFromEnv from AbstractDaemonTest#beforeTest was too late for this usage. There is no way to move it earlier in AbstractDaemonTest, because we don't have the TestNotesMigration until it's injected, which is after the schema is created. Instead, move initialization into the TestNotesMigration constructor, so we never use an uninitialized instance. Keep the setFromEnv functionality, but call it resetFromEnv. Call it after tests in case tests have modified the values during tests. Change-Id: Ib7cdcd97792edf4d278cdaea735a4fbedfd49ca3 --- .../gerrit/acceptance/AbstractDaemonTest.java | 2 +- .../gerrit/server/update/BatchUpdateTest.java | 19 ++++++++++--------- .../gerrit/testutil/GerritServerTests.java | 6 ++++-- .../gerrit/testutil/TestNotesMigration.java | 6 ++++-- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 42b10148bf..d3e084a492 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -348,7 +348,6 @@ public abstract class AbstractDaemonTest { } server.getTestInjector().injectMembers(this); - notesMigration.setFromEnv(); Transport.register(inProcessProtocol); toClose = Collections.synchronizedList(new ArrayList()); admin = accounts.admin(); @@ -528,6 +527,7 @@ public abstract class AbstractDaemonTest { server.stop(); server = null; } + notesMigration.resetFromEnv(); } protected TestRepository.CommitBuilder commitBuilder() throws Exception { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/update/BatchUpdateTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/update/BatchUpdateTest.java index 892a9ffd36..107326e221 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/update/BatchUpdateTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/update/BatchUpdateTest.java @@ -31,11 +31,13 @@ import com.google.gerrit.server.util.ThreadLocalRequestContext; import com.google.gerrit.testutil.InMemoryDatabase; import com.google.gerrit.testutil.InMemoryModule; import com.google.gerrit.testutil.InMemoryRepositoryManager; +import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Guice; import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.Provider; import com.google.inject.util.Providers; + import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; import org.eclipse.jgit.revwalk.RevCommit; @@ -45,19 +47,16 @@ import org.junit.Test; public class BatchUpdateTest { @Inject private AccountManager accountManager; - @Inject private IdentifiedUser.GenericFactory userFactory; - - @Inject private InMemoryDatabase schemaFactory; - + @Inject private SchemaFactory schemaFactory; @Inject private InMemoryRepositoryManager repoManager; - @Inject private SchemaCreator schemaCreator; - @Inject private ThreadLocalRequestContext requestContext; - @Inject private BatchUpdate.Factory batchUpdateFactory; + // Only for use in setting up/tearing down injector; other users should use schemaFactory. + @Inject private InMemoryDatabase inMemoryDatabase; + private LifecycleManager lifecycle; private ReviewDb db; private TestRepository repo; @@ -72,8 +71,10 @@ public class BatchUpdateTest { lifecycle.add(injector); lifecycle.start(); + try (ReviewDb underlyingDb = inMemoryDatabase.getDatabase().open()) { + schemaCreator.create(underlyingDb); + } db = schemaFactory.open(); - schemaCreator.create(db); Account.Id userId = accountManager.authenticate(AuthRequest.forUser("user")).getAccountId(); user = userFactory.create(userId); @@ -108,7 +109,7 @@ public class BatchUpdateTest { if (db != null) { db.close(); } - InMemoryDatabase.drop(schemaFactory); + InMemoryDatabase.drop(inMemoryDatabase); } @Test diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/GerritServerTests.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/GerritServerTests.java index 038baacdb6..dd42b67d6a 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/GerritServerTests.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/GerritServerTests.java @@ -49,8 +49,10 @@ public class GerritServerTests extends GerritBaseTests { }; public void beforeTest() throws Exception { - notesMigration = new TestNotesMigration().setFromEnv(); + notesMigration = new TestNotesMigration(); } - public void afterTest() {} + public void afterTest() { + notesMigration.resetFromEnv(); + } } diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java index d4ddaf14df..160ed8a4ff 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java @@ -29,7 +29,9 @@ public class TestNotesMigration extends NotesMigration { private volatile boolean disableChangeReviewDb; private volatile boolean failOnLoad; - public TestNotesMigration() {} + public TestNotesMigration() { + resetFromEnv(); + } @Override public boolean readChanges() { @@ -94,7 +96,7 @@ public class TestNotesMigration extends NotesMigration { return setReadChanges(enabled).setWriteChanges(enabled); } - public TestNotesMigration setFromEnv() { + public TestNotesMigration resetFromEnv() { switch (NoteDbMode.get()) { case READ_WRITE: setWriteChanges(true); From c7078ab07caab7f4c2b5340db99c5a1abdb031c4 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Mon, 24 Apr 2017 13:42:19 +0200 Subject: [PATCH 2/2] Don't fuse code and meta ref updates if it won't be atomic Prior to fusing updates in Ia5b34bae, BatchUpdate ensured that code refs updated in updateRepo were all updated before meta refs in updateChange. Fusing them is safe when BatchRefUpdate is an atomic transaction in the underlying storage, but it's actually a regression when the storage doesn't support atomic multi-ref operations. This is because all updates in the batch can fail independently, so we can end up with a meta ref update succeeding but the corresponding code update failing. To handle this case, temporarily resurrect the old NoteDbBatchUpdate implementation from 3915d7baa, switching between the fused/unfused implementations based on a new config option. It's an error to try to execute a FusedNoteDbBatchUpdate on a repository that doesn't support atomic transactions, but due to the way BatchUpdate.Factory works, we have to decide which type of update to instantiate before we have opened any repos. So we need to have a separate config option to signal this; add a new NoteDbMode FUSED to run tests in this case. To keep our tests realistic, use the atomic feature of InMemoryRepository only if we are running in FUSED mode. Setting this reveals a latent issue where we need to setAtomic(false), so fix that as well. Ideally this workaround of having multiple backends is a temporary measure until RefDirectory gains the ability to perform multi-ref transactions[1]. Long term we would like to be able to eliminate this. This somewhat depends, however, on if there are other RefDatabase implementations in the wild that people are likely to use with NoteDb that don't support atomic transactions. [1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=515678 Change-Id: I8e67dc88c7ca7d59ef3157cfb6194571a0521828 --- Documentation/dev-note-db.txt | 12 +- .../acceptance/git/AbstractPushForReview.java | 9 +- .../server/notedb/NoteDbOnlyIT.java | 4 +- .../server/notedb/ConfigNotesMigration.java | 10 + .../gerrit/server/notedb/NotesMigration.java | 13 + .../gerrit/server/project/DeleteRef.java | 1 + .../gerrit/server/update/BatchUpdate.java | 29 +- ...pdate.java => FusedNoteDbBatchUpdate.java} | 38 +- .../update/UnfusedNoteDbBatchUpdate.java | 458 ++++++++++++++++++ .../testutil/InMemoryRepositoryManager.java | 31 +- .../google/gerrit/testutil/NoteDbMode.java | 3 + .../gerrit/testutil/TestNotesMigration.java | 24 + 12 files changed, 596 insertions(+), 36 deletions(-) rename gerrit-server/src/main/java/com/google/gerrit/server/update/{NoteDbBatchUpdate.java => FusedNoteDbBatchUpdate.java} (92%) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/update/UnfusedNoteDbBatchUpdate.java diff --git a/Documentation/dev-note-db.txt b/Documentation/dev-note-db.txt index 0f6873cd61..e84fecfea1 100644 --- a/Documentation/dev-note-db.txt +++ b/Documentation/dev-note-db.txt @@ -32,8 +32,8 @@ same repository as code changes. - Storing the rest of account data is a work in progress. - Storing group data is a work in progress. -To match the current configuration of `googlesource.com`, paste the following -config snippet in your `gerrit.config`: +To use a configuration similar to the current state of `googlesource.com`, paste +the following config snippet in your `gerrit.config`: ---- [noteDb "changes"] @@ -46,6 +46,9 @@ config snippet in your `gerrit.config`: This is the configuration that will be produced if you enable experimental NoteDb support on a new site with `init`. +`googlesource.com` additionally uses `fuseUpdates = true`, because its repo +backend supports atomic multi-ref transactions. Native JGit doesn't, so setting +this option on a dev server would fail. For an example NoteDb change, poke around at this one: ---- @@ -99,6 +102,11 @@ previous options, unless otherwise noted. implementation of the `rebuild-note-db` program does not do this. + In this phase, it would be possible to delete the Changes tables out from under a running server with no effect. +- `noteDb.changes.fuseUpdates=true`: Code and meta updates within a single + repository are fused into a single atomic `BatchRefUpdate`, so they either + all succeed or all fail. This mode is only possible on a backend that + supports atomic ref updates, which notably excludes the default file repository + backend. [[migration]] == Migration diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java index 0ed5d8d25a..f99c35a04a 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -896,7 +896,14 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { PushResult pr = GitUtil.pushHead(testRepo, "refs/for/foo%base=" + rBase.getCommit().name(), false, false); - assertThat(pr.getMessages()).contains("changes: new: 1, refs: 1, done"); + + if (notesMigration.fuseUpdates()) { + // InMemoryRepository's atomic BatchRefUpdate implementation doesn't update the progress + // monitor. That's fine, we just care that there was at least one new change and no errors. + assertThat(pr.getMessages()).contains("changes: new: 1, done"); + } else { + assertThat(pr.getMessages()).contains("changes: new: 1, refs: 1, done"); + } assertTwoChangesWithSameRevision(r); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/NoteDbOnlyIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/NoteDbOnlyIT.java index ada17b610f..4a5d496017 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/NoteDbOnlyIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/NoteDbOnlyIT.java @@ -31,7 +31,6 @@ import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdateOp; import com.google.gerrit.server.update.ChangeContext; import com.google.gerrit.server.update.RepoContext; -import com.google.gerrit.testutil.NoteDbMode; import com.google.inject.Inject; import java.io.IOException; import java.util.EnumSet; @@ -48,11 +47,12 @@ public class NoteDbOnlyIT extends AbstractDaemonTest { @Before public void setUp() throws Exception { - assume().that(NoteDbMode.get()).isEqualTo(NoteDbMode.DISABLE_CHANGE_REVIEW_DB); + assume().that(notesMigration.disableChangeReviewDb()).isTrue(); } @Test public void updateChangeFailureRollsBackRefUpdate() throws Exception { + assume().that(notesMigration.fuseUpdates()).isTrue(); PushOneCommit.Result r = createChange(); Change.Id id = r.getChange().getId(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ConfigNotesMigration.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ConfigNotesMigration.java index 6ee3dfed3b..2e0439968f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ConfigNotesMigration.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ConfigNotesMigration.java @@ -48,6 +48,7 @@ public class ConfigNotesMigration extends NotesMigration { // All of these names must be reflected in the allowed set in checkConfig. private static final String DISABLE_REVIEW_DB = "disableReviewDb"; + private static final String FUSE_UPDATES = "fuseUpdates"; private static final String PRIMARY_STORAGE = "primaryStorage"; private static final String READ = "read"; private static final String SEQUENCE = "sequence"; @@ -77,6 +78,8 @@ public class ConfigNotesMigration extends NotesMigration { cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), SEQUENCE, true); cfg.setString(SECTION_NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, PrimaryStorage.NOTE_DB.name()); cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), DISABLE_REVIEW_DB, true); + // TODO(dborowitz): Set to true when FileRepository supports it. + cfg.setBoolean(SECTION_NOTE_DB, CHANGES.key(), FUSE_UPDATES, false); return cfg; } @@ -85,6 +88,7 @@ public class ConfigNotesMigration extends NotesMigration { private final boolean readChangeSequence; private final PrimaryStorage changePrimaryStorage; private final boolean disableChangeReviewDb; + private final boolean fuseUpdates; @Inject public ConfigNotesMigration(@GerritServerConfig Config cfg) { @@ -103,6 +107,7 @@ public class ConfigNotesMigration extends NotesMigration { cfg.getEnum(SECTION_NOTE_DB, CHANGES.key(), PRIMARY_STORAGE, PrimaryStorage.REVIEW_DB); disableChangeReviewDb = cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), DISABLE_REVIEW_DB, false); + fuseUpdates = cfg.getBoolean(SECTION_NOTE_DB, CHANGES.key(), FUSE_UPDATES, false); checkArgument( !(disableChangeReviewDb && changePrimaryStorage != PrimaryStorage.NOTE_DB), @@ -133,4 +138,9 @@ public class ConfigNotesMigration extends NotesMigration { public boolean disableChangeReviewDb() { return disableChangeReviewDb; } + + @Override + public boolean fuseUpdates() { + return fuseUpdates; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java index 995847ff25..7b0d76f02a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NotesMigration.java @@ -82,6 +82,19 @@ public abstract class NotesMigration { */ public abstract boolean disableChangeReviewDb(); + /** + * Fuse meta ref updates in the same batch as code updates. + * + *

When set, each {@link com.google.gerrit.server.update.BatchUpdate} results in a single + * {@link org.eclipse.jgit.lib.BatchRefUpdate} to update both code and meta refs atomically. + * Setting this option with a repository backend that does not support atomic multi-ref + * transactions ({@link org.eclipse.jgit.lib.RefDatabase#performsAtomicTransactions()}) is a + * configuration error, and all updates will fail at runtime. + * + *

Has no effect if {@link #disableChangeReviewDb()} is false. + */ + public abstract boolean fuseUpdates(); + /** * Whether to fail when reading any data from NoteDb. * diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteRef.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteRef.java index 373f56193f..78a78c2875 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteRef.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteRef.java @@ -170,6 +170,7 @@ public class DeleteRef { private void deleteMultipleRefs(Repository r) throws OrmException, IOException, ResourceConflictException { BatchRefUpdate batchUpdate = r.getRefDatabase().newBatchUpdate(); + batchUpdate.setAtomic(false); List refs = prefix == null ? refsToDelete diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/update/BatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/update/BatchUpdate.java index 03a7c375b8..22329fd9bf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/update/BatchUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/update/BatchUpdate.java @@ -94,7 +94,8 @@ public abstract class BatchUpdate implements AutoCloseable { @Override public void configure() { factory(ReviewDbBatchUpdate.AssistedFactory.class); - factory(NoteDbBatchUpdate.AssistedFactory.class); + factory(FusedNoteDbBatchUpdate.AssistedFactory.class); + factory(UnfusedNoteDbBatchUpdate.AssistedFactory.class); } }; } @@ -103,22 +104,28 @@ public abstract class BatchUpdate implements AutoCloseable { public static class Factory { private final NotesMigration migration; private final ReviewDbBatchUpdate.AssistedFactory reviewDbBatchUpdateFactory; - private final NoteDbBatchUpdate.AssistedFactory noteDbBatchUpdateFactory; + private final FusedNoteDbBatchUpdate.AssistedFactory fusedNoteDbBatchUpdateFactory; + private final UnfusedNoteDbBatchUpdate.AssistedFactory unfusedNoteDbBatchUpdateFactory; @Inject Factory( NotesMigration migration, ReviewDbBatchUpdate.AssistedFactory reviewDbBatchUpdateFactory, - NoteDbBatchUpdate.AssistedFactory noteDbBatchUpdateFactory) { + FusedNoteDbBatchUpdate.AssistedFactory fusedNoteDbBatchUpdateFactory, + UnfusedNoteDbBatchUpdate.AssistedFactory unfusedNoteDbBatchUpdateFactory) { this.migration = migration; this.reviewDbBatchUpdateFactory = reviewDbBatchUpdateFactory; - this.noteDbBatchUpdateFactory = noteDbBatchUpdateFactory; + this.fusedNoteDbBatchUpdateFactory = fusedNoteDbBatchUpdateFactory; + this.unfusedNoteDbBatchUpdateFactory = unfusedNoteDbBatchUpdateFactory; } public BatchUpdate create( ReviewDb db, Project.NameKey project, CurrentUser user, Timestamp when) { if (migration.disableChangeReviewDb()) { - return noteDbBatchUpdateFactory.create(db, project, user, when); + if (migration.fuseUpdates()) { + return fusedNoteDbBatchUpdateFactory.create(db, project, user, when); + } + return unfusedNoteDbBatchUpdateFactory.create(db, project, user, when); } return reviewDbBatchUpdateFactory.create(db, project, user, when); } @@ -138,9 +145,15 @@ public abstract class BatchUpdate implements AutoCloseable { // copy them into an ImmutableList so there is no chance the callee can pollute the input // collection. if (migration.disableChangeReviewDb()) { - ImmutableList noteDbUpdates = - (ImmutableList) ImmutableList.copyOf(updates); - NoteDbBatchUpdate.execute(noteDbUpdates, listener, requestId, dryRun); + if (migration.fuseUpdates()) { + ImmutableList noteDbUpdates = + (ImmutableList) ImmutableList.copyOf(updates); + FusedNoteDbBatchUpdate.execute(noteDbUpdates, listener, requestId, dryRun); + } else { + ImmutableList noteDbUpdates = + (ImmutableList) ImmutableList.copyOf(updates); + UnfusedNoteDbBatchUpdate.execute(noteDbUpdates, listener, requestId, dryRun); + } } else { ImmutableList reviewDbUpdates = (ImmutableList) ImmutableList.copyOf(updates); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/update/NoteDbBatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/update/FusedNoteDbBatchUpdate.java similarity index 92% rename from gerrit-server/src/main/java/com/google/gerrit/server/update/NoteDbBatchUpdate.java rename to gerrit-server/src/main/java/com/google/gerrit/server/update/FusedNoteDbBatchUpdate.java index f7988cf371..ad758484cb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/update/NoteDbBatchUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/update/FusedNoteDbBatchUpdate.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.update; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static java.util.Comparator.comparing; import com.google.common.base.Throwables; @@ -51,23 +52,25 @@ import java.util.TimeZone; import java.util.TreeMap; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevWalk; import org.eclipse.jgit.transport.ReceiveCommand; /** - * {@link BatchUpdate} implementation that only supports NoteDb. + * {@link BatchUpdate} implementation using only NoteDb that updates code refs and meta refs in a + * single {@link org.eclipse.jgit.lib.BatchRefUpdate}. * *

Used when {@code noteDb.changes.disableReviewDb=true}, at which point ReviewDb is not * consulted during updates. */ -class NoteDbBatchUpdate extends BatchUpdate { +class FusedNoteDbBatchUpdate extends BatchUpdate { interface AssistedFactory { - NoteDbBatchUpdate create( + FusedNoteDbBatchUpdate create( ReviewDb db, Project.NameKey project, CurrentUser user, Timestamp when); } static void execute( - ImmutableList updates, + ImmutableList updates, BatchUpdateListener listener, @Nullable RequestId requestId, boolean dryrun) @@ -84,11 +87,11 @@ class NoteDbBatchUpdate extends BatchUpdate { try { switch (order) { case REPO_BEFORE_DB: - for (NoteDbBatchUpdate u : updates) { + for (FusedNoteDbBatchUpdate u : updates) { u.executeUpdateRepo(); } listener.afterUpdateRepos(); - for (NoteDbBatchUpdate u : updates) { + for (FusedNoteDbBatchUpdate u : updates) { handles.add(u.executeChangeOps(dryrun)); } for (ChangesHandle h : handles) { @@ -109,10 +112,10 @@ class NoteDbBatchUpdate extends BatchUpdate { // TODO(dborowitz): This may still result in multiple updates to All-Users, but that's // currently not a big deal because multi-change batches generally aren't affecting // drafts anyway. - for (NoteDbBatchUpdate u : updates) { + for (FusedNoteDbBatchUpdate u : updates) { handles.add(u.executeChangeOps(dryrun)); } - for (NoteDbBatchUpdate u : updates) { + for (FusedNoteDbBatchUpdate u : updates) { u.executeUpdateRepo(); } for (ChangesHandle h : handles) { @@ -147,7 +150,7 @@ class NoteDbBatchUpdate extends BatchUpdate { u -> u.gitRefUpdated.fire(u.project, u.batchRefUpdate, u.getAccount().orElse(null))); if (!dryrun) { - for (NoteDbBatchUpdate u : updates) { + for (FusedNoteDbBatchUpdate u : updates) { u.executePostOps(); } } @@ -159,7 +162,7 @@ class NoteDbBatchUpdate extends BatchUpdate { class ContextImpl implements Context { @Override public RepoView getRepoView() throws IOException { - return NoteDbBatchUpdate.this.getRepoView(); + return FusedNoteDbBatchUpdate.this.getRepoView(); } @Override @@ -268,7 +271,7 @@ class NoteDbBatchUpdate extends BatchUpdate { private final ReviewDb db; @Inject - NoteDbBatchUpdate( + FusedNoteDbBatchUpdate( GitRepositoryManager repoManager, @GerritPersonIdent PersonIdent serverIdent, ChangeNotes.Factory changeNotesFactory, @@ -351,7 +354,7 @@ class NoteDbBatchUpdate extends BatchUpdate { } void execute() throws OrmException, IOException { - NoteDbBatchUpdate.this.batchRefUpdate = manager.execute(dryrun); + FusedNoteDbBatchUpdate.this.batchRefUpdate = manager.execute(dryrun); } List> startIndexFutures() { @@ -382,16 +385,19 @@ class NoteDbBatchUpdate extends BatchUpdate { private ChangesHandle executeChangeOps(boolean dryrun) throws Exception { logDebug("Executing change ops"); initRepository(); + Repository repo = repoView.getRepository(); + checkState( + repo.getRefDatabase().performsAtomicTransactions(), + "cannot use noteDb.changes.fuseUpdates=true with a repository that does not support atomic" + + " batch ref updates: %s", + repo); ChangesHandle handle = new ChangesHandle( updateManagerFactory .create(project) .setChangeRepo( - repoView.getRepository(), - repoView.getRevWalk(), - repoView.getInserter(), - repoView.getCommands()), + repo, repoView.getRevWalk(), repoView.getInserter(), repoView.getCommands()), dryrun); if (user.isIdentifiedUser()) { handle.manager.setRefLogIdent(user.asIdentifiedUser().newRefLogIdent(when, tz)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/update/UnfusedNoteDbBatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/update/UnfusedNoteDbBatchUpdate.java new file mode 100644 index 0000000000..2f7de46999 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/update/UnfusedNoteDbBatchUpdate.java @@ -0,0 +1,458 @@ +// Copyright (C) 2017 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.update; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; +import static java.util.Comparator.comparing; +import static java.util.stream.Collectors.toList; + +import com.google.common.base.Throwables; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Maps; +import com.google.common.util.concurrent.CheckedFuture; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.GerritPersonIdent; +import com.google.gerrit.server.extensions.events.GitReferenceUpdated; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.index.change.ChangeIndexer; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.notedb.ChangeUpdate; +import com.google.gerrit.server.notedb.NoteDbUpdateManager; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gerrit.server.util.RequestId; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; +import java.io.IOException; +import java.sql.Timestamp; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.TimeZone; +import java.util.TreeMap; +import org.eclipse.jgit.lib.NullProgressMonitor; +import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.ObjectReader; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevWalk; +import org.eclipse.jgit.transport.ReceiveCommand; + +/** + * {@link BatchUpdate} implementation that only supports NoteDb. + * + *

Used when {@code noteDb.changes.disableReviewDb=true}, at which point ReviewDb is not + * consulted during updates. + */ +class UnfusedNoteDbBatchUpdate extends BatchUpdate { + interface AssistedFactory { + UnfusedNoteDbBatchUpdate create( + ReviewDb db, Project.NameKey project, CurrentUser user, Timestamp when); + } + + static void execute( + ImmutableList updates, + BatchUpdateListener listener, + @Nullable RequestId requestId, + boolean dryrun) + throws UpdateException, RestApiException { + if (updates.isEmpty()) { + return; + } + setRequestIds(updates, requestId); + + try { + Order order = getOrder(updates, listener); + // TODO(dborowitz): Fuse implementations to use a single BatchRefUpdate between phases. Note + // that we may still need to respect the order, since op implementations may make assumptions + // about the order in which their methods are called. + switch (order) { + case REPO_BEFORE_DB: + for (UnfusedNoteDbBatchUpdate u : updates) { + u.executeUpdateRepo(); + } + listener.afterUpdateRepos(); + for (UnfusedNoteDbBatchUpdate u : updates) { + u.executeRefUpdates(dryrun); + } + listener.afterUpdateRefs(); + for (UnfusedNoteDbBatchUpdate u : updates) { + u.reindexChanges(u.executeChangeOps(dryrun), dryrun); + } + listener.afterUpdateChanges(); + break; + case DB_BEFORE_REPO: + for (UnfusedNoteDbBatchUpdate u : updates) { + u.reindexChanges(u.executeChangeOps(dryrun), dryrun); + } + for (UnfusedNoteDbBatchUpdate u : updates) { + u.executeUpdateRepo(); + } + for (UnfusedNoteDbBatchUpdate u : updates) { + u.executeRefUpdates(dryrun); + } + break; + default: + throw new IllegalStateException("invalid execution order: " + order); + } + + ChangeIndexer.allAsList( + updates.stream().flatMap(u -> u.indexFutures.stream()).collect(toList())) + .get(); + + // Fire ref update events only after all mutations are finished, since callers may assume a + // patch set ref being created means the change was created, or a branch advancing meaning + // some changes were closed. + updates + .stream() + .filter(u -> u.batchRefUpdate != null) + .forEach( + u -> u.gitRefUpdated.fire(u.project, u.batchRefUpdate, u.getAccount().orElse(null))); + + if (!dryrun) { + for (UnfusedNoteDbBatchUpdate u : updates) { + u.executePostOps(); + } + } + } catch (Exception e) { + wrapAndThrowException(e); + } + } + + class ContextImpl implements Context { + @Override + public RepoView getRepoView() throws IOException { + return UnfusedNoteDbBatchUpdate.this.getRepoView(); + } + + @Override + public RevWalk getRevWalk() throws IOException { + return getRepoView().getRevWalk(); + } + + @Override + public Project.NameKey getProject() { + return project; + } + + @Override + public Timestamp getWhen() { + return when; + } + + @Override + public TimeZone getTimeZone() { + return tz; + } + + @Override + public ReviewDb getDb() { + return db; + } + + @Override + public CurrentUser getUser() { + return user; + } + + @Override + public Order getOrder() { + return order; + } + } + + private class RepoContextImpl extends ContextImpl implements RepoContext { + @Override + public ObjectInserter getInserter() throws IOException { + return getRepoView().getInserterWrapper(); + } + + @Override + public void addRefUpdate(ReceiveCommand cmd) throws IOException { + getRepoView().getCommands().add(cmd); + } + } + + private class ChangeContextImpl extends ContextImpl implements ChangeContext { + private final ChangeControl ctl; + private final Map updates; + + private boolean deleted; + + protected ChangeContextImpl(ChangeControl ctl) { + this.ctl = checkNotNull(ctl); + updates = new TreeMap<>(comparing(PatchSet.Id::get)); + } + + @Override + public ChangeUpdate getUpdate(PatchSet.Id psId) { + ChangeUpdate u = updates.get(psId); + if (u == null) { + u = changeUpdateFactory.create(ctl, when); + if (newChanges.containsKey(ctl.getId())) { + u.setAllowWriteToNewRef(true); + } + u.setPatchSetId(psId); + updates.put(psId, u); + } + return u; + } + + @Override + public ChangeControl getControl() { + return ctl; + } + + @Override + public void dontBumpLastUpdatedOn() { + // Do nothing; NoteDb effectively updates timestamp if and only if a commit was written to the + // change meta ref. + } + + @Override + public void deleteChange() { + deleted = true; + } + } + + /** Per-change result status from {@link #executeChangeOps}. */ + private enum ChangeResult { + SKIPPED, + UPSERTED, + DELETED; + } + + private final ChangeNotes.Factory changeNotesFactory; + private final ChangeControl.GenericFactory changeControlFactory; + private final ChangeUpdate.Factory changeUpdateFactory; + private final NoteDbUpdateManager.Factory updateManagerFactory; + private final ChangeIndexer indexer; + private final GitReferenceUpdated gitRefUpdated; + private final ReviewDb db; + + private List> indexFutures; + + @Inject + UnfusedNoteDbBatchUpdate( + GitRepositoryManager repoManager, + @GerritPersonIdent PersonIdent serverIdent, + ChangeNotes.Factory changeNotesFactory, + ChangeControl.GenericFactory changeControlFactory, + ChangeUpdate.Factory changeUpdateFactory, + NoteDbUpdateManager.Factory updateManagerFactory, + ChangeIndexer indexer, + GitReferenceUpdated gitRefUpdated, + @Assisted ReviewDb db, + @Assisted Project.NameKey project, + @Assisted CurrentUser user, + @Assisted Timestamp when) { + super(repoManager, serverIdent, project, user, when); + checkArgument(!db.changesTablesEnabled(), "expected Change tables to be disabled on %s", db); + this.changeNotesFactory = changeNotesFactory; + this.changeControlFactory = changeControlFactory; + this.changeUpdateFactory = changeUpdateFactory; + this.updateManagerFactory = updateManagerFactory; + this.indexer = indexer; + this.gitRefUpdated = gitRefUpdated; + this.db = db; + this.indexFutures = new ArrayList<>(); + } + + @Override + public void execute(BatchUpdateListener listener) throws UpdateException, RestApiException { + execute(ImmutableList.of(this), listener, requestId, false); + } + + @Override + protected Context newContext() { + return new ContextImpl(); + } + + private void executeUpdateRepo() throws UpdateException, RestApiException { + try { + logDebug("Executing updateRepo on {} ops", ops.size()); + RepoContextImpl ctx = new RepoContextImpl(); + for (BatchUpdateOp op : ops.values()) { + op.updateRepo(ctx); + } + + logDebug("Executing updateRepo on {} RepoOnlyOps", repoOnlyOps.size()); + for (RepoOnlyOp op : repoOnlyOps) { + op.updateRepo(ctx); + } + + if (onSubmitValidators != null && !getRefUpdates().isEmpty()) { + // Validation of refs has to take place here and not at the beginning of executeRefUpdates. + // Otherwise, failing validation in a second BatchUpdate object will happen *after* the + // first update's executeRefUpdates has finished, hence after first repo's refs have been + // updated, which is too late. + onSubmitValidators.validate( + project, ctx.getRevWalk().getObjectReader(), repoView.getCommands()); + } + + // TODO(dborowitz): Don't flush when fusing phases. + if (repoView != null) { + logDebug("Flushing inserter"); + repoView.getInserter().flush(); + } else { + logDebug("No objects to flush"); + } + } catch (Exception e) { + Throwables.throwIfInstanceOf(e, RestApiException.class); + throw new UpdateException(e); + } + } + + // TODO(dborowitz): Don't execute non-change ref updates separately when fusing phases. + private void executeRefUpdates(boolean dryrun) throws IOException, RestApiException { + if (getRefUpdates().isEmpty()) { + logDebug("No ref updates to execute"); + return; + } + // May not be opened if the caller added ref updates but no new objects. + initRepository(); + batchRefUpdate = repoView.getRepository().getRefDatabase().newBatchUpdate(); + repoView.getCommands().addTo(batchRefUpdate); + logDebug("Executing batch of {} ref updates", batchRefUpdate.getCommands().size()); + if (dryrun) { + return; + } + + // Force BatchRefUpdate to read newly referenced objects using a new RevWalk, rather than one + // that might have access to unflushed objects. + try (RevWalk updateRw = new RevWalk(repoView.getRepository())) { + batchRefUpdate.execute(updateRw, NullProgressMonitor.INSTANCE); + } + boolean ok = true; + for (ReceiveCommand cmd : batchRefUpdate.getCommands()) { + if (cmd.getResult() != ReceiveCommand.Result.OK) { + ok = false; + break; + } + } + if (!ok) { + throw new RestApiException("BatchRefUpdate failed: " + batchRefUpdate); + } + } + + private Map executeChangeOps(boolean dryrun) throws Exception { + logDebug("Executing change ops"); + Map result = + Maps.newLinkedHashMapWithExpectedSize(ops.keySet().size()); + initRepository(); + Repository repo = repoView.getRepository(); + // TODO(dborowitz): Teach NoteDbUpdateManager to allow reusing the same inserter and batch ref + // update as in executeUpdateRepo. + try (ObjectInserter ins = repo.newObjectInserter(); + ObjectReader reader = ins.newReader(); + RevWalk rw = new RevWalk(reader); + NoteDbUpdateManager updateManager = + updateManagerFactory + .create(project) + .setChangeRepo(repo, rw, ins, new ChainedReceiveCommands(repo))) { + if (user.isIdentifiedUser()) { + updateManager.setRefLogIdent(user.asIdentifiedUser().newRefLogIdent(when, tz)); + } + for (Map.Entry> e : ops.asMap().entrySet()) { + Change.Id id = e.getKey(); + ChangeContextImpl ctx = newChangeContext(id); + boolean dirty = false; + logDebug("Applying {} ops for change {}", e.getValue().size(), id); + for (BatchUpdateOp op : e.getValue()) { + dirty |= op.updateChange(ctx); + } + if (!dirty) { + logDebug("No ops reported dirty, short-circuiting"); + result.put(id, ChangeResult.SKIPPED); + continue; + } + for (ChangeUpdate u : ctx.updates.values()) { + updateManager.add(u); + } + if (ctx.deleted) { + logDebug("Change {} was deleted", id); + updateManager.deleteChange(id); + result.put(id, ChangeResult.DELETED); + } else { + result.put(id, ChangeResult.UPSERTED); + } + } + + if (!dryrun) { + logDebug("Executing NoteDb updates"); + updateManager.execute(); + } + } + return result; + } + + private ChangeContextImpl newChangeContext(Change.Id id) throws OrmException { + logDebug("Opening change {} for update", id); + Change c = newChanges.get(id); + boolean isNew = c != null; + if (!isNew) { + // Pass a synthetic change into ChangeNotes.Factory, which will take care of checking for + // existence and populating columns from the parsed notes state. + // TODO(dborowitz): This dance made more sense when using Reviewdb; consider a nicer way. + c = ChangeNotes.Factory.newNoteDbOnlyChange(project, id); + } else { + logDebug("Change {} is new", id); + } + ChangeNotes notes = changeNotesFactory.createForBatchUpdate(c, !isNew); + ChangeControl ctl = changeControlFactory.controlFor(notes, user); + return new ChangeContextImpl(ctl); + } + + private void reindexChanges(Map updateResults, boolean dryrun) { + if (dryrun) { + return; + } + logDebug("Reindexing {} changes", updateResults.size()); + for (Map.Entry e : updateResults.entrySet()) { + Change.Id id = e.getKey(); + switch (e.getValue()) { + case UPSERTED: + indexFutures.add(indexer.indexAsync(project, id)); + break; + case DELETED: + indexFutures.add(indexer.deleteAsync(id)); + break; + case SKIPPED: + break; + default: + throw new IllegalStateException("unexpected result: " + e.getValue()); + } + } + } + + private void executePostOps() throws Exception { + ContextImpl ctx = new ContextImpl(); + for (BatchUpdateOp op : ops.values()) { + op.postUpdate(ctx); + } + + for (RepoOnlyOp op : repoOnlyOps) { + op.postUpdate(ctx); + } + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java index 4826d9e3e9..a90cbb97ea 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryRepositoryManager.java @@ -19,6 +19,8 @@ import com.google.common.collect.Sets; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.RepositoryCaseMismatchException; +import com.google.gerrit.server.notedb.NotesMigration; +import com.google.inject.Inject; import java.util.HashMap; import java.util.Map; import java.util.SortedSet; @@ -30,7 +32,7 @@ import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; /** Repository manager that uses in-memory repositories. */ public class InMemoryRepositoryManager implements GitRepositoryManager { public static InMemoryRepository newRepository(Project.NameKey name) { - return new Repo(name); + return new Repo(new TestNotesMigration(), name); } public static class Description extends DfsRepositoryDescription { @@ -49,11 +51,15 @@ public class InMemoryRepositoryManager implements GitRepositoryManager { public static class Repo extends InMemoryRepository { private String description; - private Repo(Project.NameKey name) { + private Repo(NotesMigration notesMigration, Project.NameKey name) { super(new Description(name)); - // TODO(dborowitz): Allow atomic transactions when this is supported: - // https://git.eclipse.org/r/#/c/61841/2/org.eclipse.jgit/src/org/eclipse/jgit/internal/storage/dfs/InMemoryRepository.java@313 - setPerformsAtomicTransactions(false); + // Normally, mimic the behavior of JGit FileRepository, the standard Gerrit repository + // backend, and don't support atomic ref updates. The exception is when we're testing with + // fused ref updates, which requires atomic ref updates to function. + // + // TODO(dborowitz): Change to match the behavior of JGit FileRepository after fixing + // https://bugs.eclipse.org/bugs/show_bug.cgi?id=515678 + setPerformsAtomicTransactions(notesMigration.fuseUpdates()); } @Override @@ -72,7 +78,18 @@ public class InMemoryRepositoryManager implements GitRepositoryManager { } } - private Map repos = new HashMap<>(); + private final NotesMigration notesMigration; + private final Map repos; + + public InMemoryRepositoryManager() { + this(new TestNotesMigration()); + } + + @Inject + InMemoryRepositoryManager(NotesMigration notesMigration) { + this.notesMigration = notesMigration; + this.repos = new HashMap<>(); + } @Override public synchronized Repo openRepository(Project.NameKey name) throws RepositoryNotFoundException { @@ -89,7 +106,7 @@ public class InMemoryRepositoryManager implements GitRepositoryManager { throw new RepositoryCaseMismatchException(name); } } catch (RepositoryNotFoundException e) { - repo = new Repo(name); + repo = new Repo(notesMigration, name); repos.put(normalize(name), repo); } return repo; diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java index 552f6f105f..e8446a20b0 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/NoteDbMode.java @@ -35,6 +35,9 @@ public enum NoteDbMode { /** All change tables are entirely disabled. */ DISABLE_CHANGE_REVIEW_DB(true), + /** All change tables are entirely disabled, and code/meta ref updates are fused. */ + FUSED(true), + /** * Run tests with NoteDb disabled, then convert ReviewDb to NoteDb and check that the results * match. diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java index 160ed8a4ff..c05dd01994 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestNotesMigration.java @@ -27,6 +27,7 @@ public class TestNotesMigration extends NotesMigration { private volatile boolean writeChanges; private volatile PrimaryStorage changePrimaryStorage = PrimaryStorage.REVIEW_DB; private volatile boolean disableChangeReviewDb; + private volatile boolean fuseUpdates; private volatile boolean failOnLoad; public TestNotesMigration() { @@ -55,6 +56,11 @@ public class TestNotesMigration extends NotesMigration { return disableChangeReviewDb; } + @Override + public boolean fuseUpdates() { + return fuseUpdates; + } + // Increase visbility from superclass, as tests may want to check whether // NoteDb data is written in specific migration scenarios. @Override @@ -87,6 +93,11 @@ public class TestNotesMigration extends NotesMigration { return this; } + public TestNotesMigration setFuseUpdates(boolean fuseUpdates) { + this.fuseUpdates = fuseUpdates; + return this; + } + public TestNotesMigration setFailOnLoad(boolean failOnLoad) { this.failOnLoad = failOnLoad; return this; @@ -103,24 +114,35 @@ public class TestNotesMigration extends NotesMigration { setReadChanges(true); setChangePrimaryStorage(PrimaryStorage.REVIEW_DB); setDisableChangeReviewDb(false); + setFuseUpdates(false); break; case WRITE: setWriteChanges(true); setReadChanges(false); setChangePrimaryStorage(PrimaryStorage.REVIEW_DB); setDisableChangeReviewDb(false); + setFuseUpdates(false); break; case PRIMARY: setWriteChanges(true); setReadChanges(true); setChangePrimaryStorage(PrimaryStorage.NOTE_DB); setDisableChangeReviewDb(false); + setFuseUpdates(false); break; case DISABLE_CHANGE_REVIEW_DB: setWriteChanges(true); setReadChanges(true); setChangePrimaryStorage(PrimaryStorage.NOTE_DB); setDisableChangeReviewDb(true); + setFuseUpdates(false); + break; + case FUSED: + setWriteChanges(true); + setReadChanges(true); + setChangePrimaryStorage(PrimaryStorage.NOTE_DB); + setDisableChangeReviewDb(true); + setFuseUpdates(true); break; case CHECK: case OFF: @@ -129,6 +151,7 @@ public class TestNotesMigration extends NotesMigration { setReadChanges(false); setChangePrimaryStorage(PrimaryStorage.REVIEW_DB); setDisableChangeReviewDb(false); + setFuseUpdates(false); break; } return this; @@ -139,6 +162,7 @@ public class TestNotesMigration extends NotesMigration { setReadChanges(other.readChanges()); setChangePrimaryStorage(other.changePrimaryStorage()); setDisableChangeReviewDb(other.disableChangeReviewDb()); + setFuseUpdates(other.fuseUpdates()); return this; } }