From 15ca7b0bcb8782a9d493e3abb8df669c59debfd6 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Wed, 15 Jun 2016 17:58:58 +0200 Subject: [PATCH] Migrate reviewed flags to local H2 database With NoteDb Gerrit will no longer have a database. Hence all data from ReviewDb must be migrated away from it. This change migrates the reviewed flags from ReviewDb to a local H2 database. Reviewed flags are tuples of (patch set ID, file, account ID) and record whether a user has reviewed a file in a patch set. Each user can easily have thousands of reviewed flags and the number of reviewed flags is growing without bound. Most data from ReviewDb is migrated to be stored in git, but for reviewed flags a git-based storage is not very well suitable because there is a relatively high write latency [1]. Also depending on the exact storage format updating reviewed flags may create a huge git history which is not good for anything since an audit log for reviewed flags is not needed [1]. Since reviewed flags are small per-user bits that are quickly read and updated a database seems to be the best storage [2]. This is why with this change the reviewed flags will be stored in a local H2 database. This store implements the AccountPatchReviewStore extension point that was added by change Iaaacd9f0a. Plugins can implement this extension point and replace the local H2 database with another storage. To support multi-master the local H2 database must be replaced by a storage that supports replication, e.g. a MySQL replication system [2]. This can be done by implementing the AccountPatchReviewStore extension point. A schema migration copies all reviewed flags from ReviewDb to the local H2 database and the old ACCOUNT_PATCH_REVIEWS table in ReviewDb is dropped. Not using gwtorm to access the local H2 database is intentional. When ReviewDb is gone, we want to drop the dependency to gwtorm, and hence we don't want to introduce any new dependencies to it. Instead the implementation is using raw SQL. The local H2 database is stored in the review site in: $site_path/db/account_patch_reviews.h2.db For the acceptence tests an in memory H2 database is used. [1] https://groups.google.com/d/msg/repo-discuss/KhXKMKTJNMs/Tq3XaB8wCgAJ [2] https://groups.google.com/d/msg/repo-discuss/KhXKMKTJNMs/hHoCa1_5CwAJ Change-Id: I7cb77ff124e36709cc69ef547f2b4e5e1a1d7f62 Signed-off-by: Edwin Kempin --- Documentation/config-gerrit.txt | 7 +- .../java/com/google/gerrit/pgm/Daemon.java | 6 +- .../server/AccountPatchReviewAccess.java | 37 --- .../gerrit/reviewdb/server/ReviewDb.java | 3 +- .../reviewdb/server/ReviewDbWrapper.java | 5 - .../change/AccountPatchReviewStoreImpl.java | 132 --------- .../server/config/GerritGlobalModule.java | 2 + .../gerrit/server/config/SitePaths.java | 2 + .../com/google/gerrit/server/schema/H2.java | 24 +- .../schema/H2AccountPatchReviewStore.java | 269 ++++++++++++++++++ .../gerrit/server/schema/SchemaVersion.java | 2 +- .../gerrit/server/schema/Schema_127.java | 75 +++++ .../gerrit/testutil/DisabledReviewDb.java | 6 - .../gerrit/testutil/InMemoryModule.java | 4 +- .../gerrit/httpd/WebAppInitializer.java | 4 +- 15 files changed, 386 insertions(+), 192 deletions(-) delete mode 100644 gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountPatchReviewAccess.java delete mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/AccountPatchReviewStoreImpl.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/schema/H2AccountPatchReviewStore.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_127.java diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 621325ee95..18a7a1f8ed 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -539,9 +539,8 @@ Default is unset, no disk cache. [[cache.h2CacheSize]]cache.h2CacheSize:: + -The size of the H2 database cache, in bytes, used for each persistent cache. +The size of the in-memory cache for each opened H2 database, in bytes. + -Some caches of Gerrit are persistent and are backed by an H2 database. H2 uses memory to cache its database content. The parameter `h2CacheSize` allows to limit the memory used by H2 and thus prevent out-of-memory caused by the H2 database using too much memory. @@ -550,6 +549,10 @@ Technically the H2 cache size is configured using the CACHE_SIZE parameter in the H2 JDBC connection URL, as described link:http://www.h2database.com/html/features.html#cache_settings[here] + +Gerrit uses H2 for storing reviewed flags on changes and for persistent +caches. The configured cache size is used for each of these local H2 +databases. ++ Default is unset, no cache size limit. + Common unit suffixes of 'k', 'm', or 'g' are supported. diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java index 015b4d29df..4715877563 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java @@ -52,7 +52,6 @@ import com.google.gerrit.pgm.util.SiteProgram; import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; -import com.google.gerrit.server.change.AccountPatchReviewStoreImpl; import com.google.gerrit.server.change.ChangeCleanupRunner; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfigModule; @@ -76,6 +75,7 @@ import com.google.gerrit.server.patch.DiffExecutorModule; import com.google.gerrit.server.plugins.PluginGuiceEnvironment; import com.google.gerrit.server.plugins.PluginRestApiModule; import com.google.gerrit.server.schema.DataSourceProvider; +import com.google.gerrit.server.schema.H2AccountPatchReviewStore; import com.google.gerrit.server.schema.SchemaVersionCheck; import com.google.gerrit.server.securestore.DefaultSecureStore; import com.google.gerrit.server.securestore.SecureStore; @@ -347,7 +347,9 @@ public class Daemon extends SiteProgram { modules.add(new StreamEventsApiListener.Module()); modules.add(new ChangeHookRunner.Module()); modules.add(new EventBroker.Module()); - modules.add(new AccountPatchReviewStoreImpl.Module()); + modules.add(test + ? new H2AccountPatchReviewStore.InMemoryModule() + : new H2AccountPatchReviewStore.Module()); modules.add(new ReceiveCommitsExecutorModule()); modules.add(new DiffExecutorModule()); modules.add(new MimeUtil2Module()); diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountPatchReviewAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountPatchReviewAccess.java deleted file mode 100644 index 6b9e16097b..0000000000 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/AccountPatchReviewAccess.java +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (C) 2009 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.reviewdb.server; - -import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.AccountPatchReview; -import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gwtorm.server.Access; -import com.google.gwtorm.server.OrmException; -import com.google.gwtorm.server.PrimaryKey; -import com.google.gwtorm.server.Query; -import com.google.gwtorm.server.ResultSet; - -public interface AccountPatchReviewAccess - extends Access { - @Override - @PrimaryKey("key") - AccountPatchReview get(AccountPatchReview.Key id) throws OrmException; - - @Query("WHERE key.accountId = ? AND key.patchKey.patchSetId = ?") - ResultSet byReviewer(Account.Id who, PatchSet.Id ps) throws OrmException; - - @Query("WHERE key.patchKey.patchSetId = ?") - ResultSet byPatchSet(PatchSet.Id ps) throws OrmException; -} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java index 9f62dc2450..c585ca5f6a 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDb.java @@ -74,8 +74,7 @@ public interface ReviewDb extends Schema { @Relation(id = 19) AccountProjectWatchAccess accountProjectWatches(); - @Relation(id = 20) - AccountPatchReviewAccess accountPatchReviews(); + // Deleted @Relation(id = 20) @Relation(id = 21) ChangeAccess changes(); diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java index aa3797466d..6b25378537 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java @@ -113,11 +113,6 @@ public class ReviewDbWrapper implements ReviewDb { return delegate.accountProjectWatches(); } - @Override - public AccountPatchReviewAccess accountPatchReviews() { - return delegate.accountPatchReviews(); - } - @Override public ChangeAccess changes() { return delegate.changes(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/AccountPatchReviewStoreImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/AccountPatchReviewStoreImpl.java deleted file mode 100644 index 16dd130a99..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/AccountPatchReviewStoreImpl.java +++ /dev/null @@ -1,132 +0,0 @@ -// Copyright (C) 2016 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.change; - -import com.google.common.base.Function; -import com.google.common.collect.Collections2; -import com.google.common.collect.Iterables; -import com.google.gerrit.extensions.registration.DynamicItem; -import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.AccountPatchReview; -import com.google.gerrit.reviewdb.client.Patch; -import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gwtorm.server.OrmDuplicateKeyException; -import com.google.gwtorm.server.OrmException; -import com.google.inject.AbstractModule; -import com.google.inject.Inject; -import com.google.inject.Provider; - -import java.util.Collection; -import java.util.Collections; - -import javax.inject.Singleton; - -@Singleton -public class AccountPatchReviewStoreImpl implements AccountPatchReviewStore { - private final Provider dbProvider; - - public static class Module extends AbstractModule { - @Override - protected void configure() { - DynamicItem.itemOf(binder(), AccountPatchReviewStore.class); - DynamicItem.bind(binder(), AccountPatchReviewStore.class) - .to(AccountPatchReviewStoreImpl.class); - } - } - - @Inject - AccountPatchReviewStoreImpl(Provider dbProvider) { - this.dbProvider = dbProvider; - } - - @Override - public boolean markReviewed(PatchSet.Id psId, Account.Id accountId, - String path) throws OrmException { - ReviewDb db = dbProvider.get(); - AccountPatchReview apr = getExisting(db, psId, path, accountId); - if (apr != null) { - return false; - } - - try { - db.accountPatchReviews().insert(Collections.singleton( - new AccountPatchReview(new Patch.Key(psId, path), accountId))); - return true; - } catch (OrmDuplicateKeyException e) { - // Ignored - return false; - } - } - - @Override - public void markReviewed(final PatchSet.Id psId, final Account.Id accountId, - final Collection paths) throws OrmException { - if (paths == null || paths.isEmpty()) { - return; - } else if (paths.size() == 1) { - markReviewed(psId, accountId, Iterables.getOnlyElement(paths)); - return; - } - - paths.removeAll(findReviewed(psId, accountId)); - if (paths.isEmpty()) { - return; - } - dbProvider.get().accountPatchReviews().insert(Collections2.transform(paths, - new Function() { - @Override - public AccountPatchReview apply(String path) { - return new AccountPatchReview(new Patch.Key(psId, path), accountId); - } - })); - } - - @Override - public void clearReviewed(PatchSet.Id psId, Account.Id accountId, String path) - throws OrmException { - ReviewDb db = dbProvider.get(); - AccountPatchReview apr = getExisting(db, psId, path, accountId); - if (apr != null) { - db.accountPatchReviews().delete(Collections.singleton(apr)); - } - } - - @Override - public void clearReviewed(PatchSet.Id psId) throws OrmException { - dbProvider.get().accountPatchReviews() - .delete(dbProvider.get().accountPatchReviews().byPatchSet(psId)); - } - - @Override - public Collection findReviewed(PatchSet.Id psId, Account.Id accountId) - throws OrmException { - return Collections2.transform(dbProvider.get().accountPatchReviews() - .byReviewer(accountId, psId).toList(), - new Function() { - @Override - public String apply(AccountPatchReview apr) { - return apr.getKey().getPatchKey().getFileName(); - } - }); - } - - private static AccountPatchReview getExisting(ReviewDb db, PatchSet.Id psId, - String path, Account.Id accountId) throws OrmException { - AccountPatchReview.Key key = - new AccountPatchReview.Key(new Patch.Key(psId, path), accountId); - return db.accountPatchReviews().get(key); - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index 6b19d1a151..9781f39de5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -91,6 +91,7 @@ import com.google.gerrit.server.auth.UniversalAuthBackend; import com.google.gerrit.server.auth.oauth.OAuthTokenCache; import com.google.gerrit.server.avatar.AvatarProvider; import com.google.gerrit.server.cache.CacheRemovalListener; +import com.google.gerrit.server.change.AccountPatchReviewStore; import com.google.gerrit.server.change.ChangeJson; import com.google.gerrit.server.change.ChangeKindCacheImpl; import com.google.gerrit.server.change.MergeabilityCacheImpl; @@ -348,6 +349,7 @@ public class GerritGlobalModule extends FactoryModule { DynamicItem.itemOf(binder(), OAuthTokenEncrypter.class); DynamicSet.setOf(binder(), AccountExternalIdCreator.class); DynamicSet.setOf(binder(), WebUiPlugin.class); + DynamicItem.itemOf(binder(), AccountPatchReviewStore.class); factory(UploadValidators.Factory.class); DynamicSet.setOf(binder(), UploadValidationListener.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java index e41fb30f31..192ca49cbc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/SitePaths.java @@ -38,6 +38,7 @@ public final class SitePaths { public final Path tmp_dir; public final Path logs_dir; public final Path plugins_dir; + public final Path db_dir; public final Path data_dir; public final Path mail_dir; public final Path hooks_dir; @@ -75,6 +76,7 @@ public final class SitePaths { lib_dir = p.resolve("lib"); tmp_dir = p.resolve("tmp"); plugins_dir = p.resolve("plugins"); + db_dir = p.resolve("db"); data_dir = p.resolve("data"); logs_dir = p.resolve("logs"); mail_dir = etc_dir.resolve("mail"); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/H2.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/H2.java index 66f2f1d7a6..3bec395869 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/H2.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/H2.java @@ -20,6 +20,8 @@ import com.google.inject.Inject; import org.eclipse.jgit.lib.Config; +import java.nio.file.Path; + class H2 extends BaseDataSourceType { protected final Config cfg; @@ -38,6 +40,26 @@ class H2 extends BaseDataSourceType { if (database == null || database.isEmpty()) { database = "db/ReviewDB"; } - return "jdbc:h2:" + site.resolve(database).toUri().toString(); + return createUrl(site.resolve(database)); + } + + public static String createUrl(Path path) { + return new StringBuilder() + .append("jdbc:h2:") + .append(path.toUri().toString()) + .toString(); + } + + public static String appendCacheSize(Config cfg, String url) { + long h2CacheSize = cfg.getLong("cache", null, "h2CacheSize", -1); + if (h2CacheSize >= 0) { + // H2 CACHE_SIZE is always given in KB + return new StringBuilder() + .append(url) + .append(";CACHE_SIZE=") + .append(h2CacheSize / 1024) + .toString(); + } + return url; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/H2AccountPatchReviewStore.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/H2AccountPatchReviewStore.java new file mode 100644 index 0000000000..8e74f6c855 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/H2AccountPatchReviewStore.java @@ -0,0 +1,269 @@ +// Copyright (C) 2016 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.schema; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.primitives.Ints; +import com.google.gerrit.extensions.events.LifecycleListener; +import com.google.gerrit.extensions.registration.DynamicItem; +import com.google.gerrit.lifecycle.LifecycleModule; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.server.change.AccountPatchReviewStore; +import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.config.SitePaths; +import com.google.gwtorm.server.OrmDuplicateKeyException; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; + +import org.eclipse.jgit.lib.Config; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import javax.inject.Singleton; + +@Singleton +public class H2AccountPatchReviewStore + implements AccountPatchReviewStore, LifecycleListener { + private static final Logger log = + LoggerFactory.getLogger(H2AccountPatchReviewStore.class); + + public static class Module extends LifecycleModule { + @Override + protected void configure() { + DynamicItem.bind(binder(), AccountPatchReviewStore.class) + .to(H2AccountPatchReviewStore.class); + listener().to(H2AccountPatchReviewStore.class); + } + } + + @VisibleForTesting + public static class InMemoryModule extends LifecycleModule { + @Override + protected void configure() { + H2AccountPatchReviewStore inMemoryStore = new H2AccountPatchReviewStore(); + DynamicItem.bind(binder(), AccountPatchReviewStore.class) + .toInstance(inMemoryStore); + listener().toInstance(inMemoryStore); + } + } + + private final String url; + + @Inject + H2AccountPatchReviewStore(@GerritServerConfig Config cfg, + SitePaths sitePaths) { + this.url = H2.appendCacheSize(cfg, getUrl(sitePaths)); + } + + public static String getUrl(SitePaths sitePaths) { + return H2.createUrl(sitePaths.db_dir.resolve("account_patch_reviews")); + } + + /** + * Creates an in-memory H2 database to store the reviewed flags. + * This should be used for tests only. + */ + @VisibleForTesting + private H2AccountPatchReviewStore() { + // DB_CLOSE_DELAY=-1: By default the content of an in-memory H2 database is + // lost at the moment the last connection is closed. This option keeps the + // content as long as the vm lives. + this.url = "jdbc:h2:mem:account_patch_reviews;DB_CLOSE_DELAY=-1"; + } + + @Override + public void start() { + try { + createTableIfNotExists(url); + } catch (OrmException e) { + log.error("Failed to create table to store account patch reviews", e); + } + } + + public static void createTableIfNotExists(String url) throws OrmException { + try (Connection con = DriverManager.getConnection(url); + Statement stmt = con.createStatement()) { + stmt.executeUpdate("CREATE TABLE IF NOT EXISTS ACCOUNT_PATCH_REVIEWS (" + + "ACCOUNT_ID INTEGER DEFAULT 0 NOT NULL, " + + "CHANGE_ID INTEGER DEFAULT 0 NOT NULL, " + + "PATCH_SET_ID INTEGER DEFAULT 0 NOT NULL, " + + "FILE_NAME VARCHAR(255) DEFAULT '' NOT NULL, " + + "CONSTRAINT PRIMARY_KEY_ACCOUNT_PATCH_REVIEWS " + + "PRIMARY KEY (ACCOUNT_ID, CHANGE_ID, PATCH_SET_ID, FILE_NAME)" + + ")"); + } catch (SQLException e) { + throw convertError("create", e); + } + } + + public static void dropTableIfExists(String url) throws OrmException { + try (Connection con = DriverManager.getConnection(url); + Statement stmt = con.createStatement()) { + stmt.executeUpdate("DROP TABLE IF EXISTS ACCOUNT_PATCH_REVIEWS"); + } catch (SQLException e) { + throw convertError("create", e); + } + } + + @Override + public void stop() { + } + + @Override + public boolean markReviewed(PatchSet.Id psId, Account.Id accountId, + String path) throws OrmException { + try (Connection con = DriverManager.getConnection(url); + PreparedStatement stmt = + con.prepareStatement("INSERT INTO ACCOUNT_PATCH_REVIEWS " + + "(ACCOUNT_ID, CHANGE_ID, PATCH_SET_ID, FILE_NAME) VALUES " + + "(?, ?, ?, ?)")) { + stmt.setInt(1, accountId.get()); + stmt.setInt(2, psId.getParentKey().get()); + stmt.setInt(3, psId.get()); + stmt.setString(4, path); + stmt.executeUpdate(); + return true; + } catch (SQLException e) { + OrmException ormException = convertError("insert", e); + if (ormException instanceof OrmDuplicateKeyException) { + return false; + } + throw ormException; + } + } + + @Override + public void markReviewed(PatchSet.Id psId, Account.Id accountId, + Collection paths) throws OrmException { + if (paths == null || paths.isEmpty()) { + return; + } + + try (Connection con = DriverManager.getConnection(url); + PreparedStatement stmt = + con.prepareStatement("INSERT INTO ACCOUNT_PATCH_REVIEWS " + + "(ACCOUNT_ID, CHANGE_ID, PATCH_SET_ID, FILE_NAME) VALUES " + + "(?, ?, ?, ?)")) { + for (String path : paths) { + stmt.setInt(1, accountId.get()); + stmt.setInt(2, psId.getParentKey().get()); + stmt.setInt(3, psId.get()); + stmt.setString(4, path); + stmt.addBatch(); + } + stmt.executeBatch(); + } catch (SQLException e) { + throw convertError("insert", e); + } + } + + @Override + public void clearReviewed(PatchSet.Id psId, Account.Id accountId, String path) + throws OrmException { + try (Connection con = DriverManager.getConnection(url); + PreparedStatement stmt = + con.prepareStatement("DELETE FROM ACCOUNT_PATCH_REVIEWS " + + "WHERE ACCOUNT_ID = ? AND CHANGE_ID + ? AND " + + "PATCH_SET_ID = ? AND FILE_NAME = ?")) { + stmt.setInt(1, accountId.get()); + stmt.setInt(2, psId.getParentKey().get()); + stmt.setInt(3, psId.get()); + stmt.setString(4, path); + stmt.executeUpdate(); + } catch (SQLException e) { + throw convertError("delete", e); + } + } + + @Override + public void clearReviewed(PatchSet.Id psId) throws OrmException { + try (Connection con = DriverManager.getConnection(url); + PreparedStatement stmt = + con.prepareStatement("DELETE FROM ACCOUNT_PATCH_REVIEWS " + + "WHERE CHANGE_ID + ? AND PATCH_SET_ID = ?")) { + stmt.setInt(1, psId.getParentKey().get()); + stmt.setInt(2, psId.get()); + stmt.executeUpdate(); + } catch (SQLException e) { + throw convertError("delete", e); + } + } + + @Override + public Collection findReviewed(PatchSet.Id psId, Account.Id accountId) + throws OrmException { + try (Connection con = DriverManager.getConnection(url); + PreparedStatement stmt = + con.prepareStatement("SELECT FILE_NAME FROM ACCOUNT_PATCH_REVIEWS " + + "WHERE ACCOUNT_ID = ? AND CHANGE_ID = ? AND PATCH_SET_ID = ?")) { + stmt.setInt(1, accountId.get()); + stmt.setInt(2, psId.getParentKey().get()); + stmt.setInt(3, psId.get()); + try (ResultSet rs = stmt.executeQuery()) { + List files = new ArrayList<>(); + while (rs.next()) { + files.add(rs.getString("FILE_NAME")); + } + return files; + } + } catch (SQLException e) { + throw convertError("select", e); + } + } + + public static OrmException convertError(String op, SQLException err) { + switch (getSQLStateInt(err)) { + case 23001: // UNIQUE CONSTRAINT VIOLATION + case 23505: // DUPLICATE_KEY_1 + return new OrmDuplicateKeyException("ACCOUNT_PATCH_REVIEWS", err); + + default: + if (err.getCause() == null && err.getNextException() != null) { + err.initCause(err.getNextException()); + } + return new OrmException(op + " failure on ACCOUNT_PATCH_REVIEWS", err); + } + } + + private static String getSQLState(SQLException err) { + String ec; + SQLException next = err; + do { + ec = next.getSQLState(); + next = next.getNextException(); + } while (ec == null && next != null); + return ec; + } + + private static int getSQLStateInt(SQLException err) { + String s = getSQLState(err); + if (s != null) { + Integer i = Ints.tryParse(s); + return i != null ? i : -1; + } + return 0; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index 2871dde77e..6ad2fb8430 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java @@ -33,7 +33,7 @@ import java.util.List; /** A version of the database schema. */ public abstract class SchemaVersion { /** The current schema version. */ - public static final Class C = Schema_126.class; + public static final Class C = Schema_127.class; public static int getBinaryVersion() { return guessVersion(C); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_127.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_127.java new file mode 100644 index 0000000000..b9e4bfa19f --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_127.java @@ -0,0 +1,75 @@ +// Copyright (C) 2016 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.schema; + +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.config.SitePaths; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; + +public class Schema_127 extends SchemaVersion { + private static final int MAX_BATCH_SIZE = 1000; + + private final SitePaths sitePaths; + + @Inject + Schema_127(Provider prior, SitePaths sitePaths) { + super(prior); + this.sitePaths = sitePaths; + } + + @Override + protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException { + String url = H2AccountPatchReviewStore.getUrl(sitePaths); + H2AccountPatchReviewStore.dropTableIfExists(url); + H2AccountPatchReviewStore.createTableIfNotExists(url); + try (Connection con = DriverManager.getConnection(url); + PreparedStatement stmt = + con.prepareStatement("INSERT INTO ACCOUNT_PATCH_REVIEWS " + + "(ACCOUNT_ID, CHANGE_ID, PATCH_SET_ID, FILE_NAME) VALUES " + + "(?, ?, ?, ?)")) { + int batchCount = 0; + + try (Statement s = newStatement(db); + ResultSet rs = s.executeQuery("SELECT * from ACCOUNT_PATCH_REVIEWS")) { + while (rs.next()) { + stmt.setInt(1, rs.getInt("ACCOUNT_ID")); + stmt.setInt(2, rs.getInt("CHANGE_ID")); + stmt.setInt(3, rs.getInt("PATCH_SET_ID")); + stmt.setString(4, rs.getString("FILE_NAME")); + stmt.addBatch(); + batchCount++; + if (batchCount >= MAX_BATCH_SIZE) { + stmt.executeBatch(); + batchCount = 0; + } + } + } + if (batchCount > 0) { + stmt.executeBatch(); + } + } catch (SQLException e) { + throw H2AccountPatchReviewStore.convertError("insert", e); + } + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java index ad9a46a926..11d7ad030f 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/DisabledReviewDb.java @@ -22,7 +22,6 @@ import com.google.gerrit.reviewdb.server.AccountGroupByIdAudAccess; import com.google.gerrit.reviewdb.server.AccountGroupMemberAccess; import com.google.gerrit.reviewdb.server.AccountGroupMemberAuditAccess; import com.google.gerrit.reviewdb.server.AccountGroupNameAccess; -import com.google.gerrit.reviewdb.server.AccountPatchReviewAccess; import com.google.gerrit.reviewdb.server.AccountProjectWatchAccess; import com.google.gerrit.reviewdb.server.ChangeAccess; import com.google.gerrit.reviewdb.server.ChangeMessageAccess; @@ -120,11 +119,6 @@ public class DisabledReviewDb implements ReviewDb { throw new Disabled(); } - @Override - public AccountPatchReviewAccess accountPatchReviews() { - throw new Disabled(); - } - @Override public ChangeAccess changes() { throw new Disabled(); diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java index 2f5de15cb6..9f1f031ff7 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java @@ -29,7 +29,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdentProvider; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; -import com.google.gerrit.server.change.AccountPatchReviewStoreImpl; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.AllProjectsNameProvider; import com.google.gerrit.server.config.AllUsersName; @@ -56,6 +55,7 @@ import com.google.gerrit.server.mail.SignedTokenEmailTokenVerifier; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.patch.DiffExecutor; import com.google.gerrit.server.schema.DataSourceType; +import com.google.gerrit.server.schema.H2AccountPatchReviewStore; import com.google.gerrit.server.schema.SchemaCreator; import com.google.gerrit.server.securestore.DefaultSecureStore; import com.google.gerrit.server.securestore.SecureStore; @@ -204,7 +204,7 @@ public class InMemoryModule extends FactoryModule { install(new FakeEmailSender.Module()); install(new SignedTokenEmailTokenVerifier.Module()); install(new GpgModule(cfg)); - install(new AccountPatchReviewStoreImpl.Module()); + install(new H2AccountPatchReviewStore.InMemoryModule()); IndexType indexType = null; try { diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java index 9c1bab8de9..64dce26f7b 100644 --- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java +++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java @@ -34,7 +34,6 @@ import com.google.gerrit.metrics.dropwizard.DropWizardMetricMaker; import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; -import com.google.gerrit.server.change.AccountPatchReviewStoreImpl; import com.google.gerrit.server.change.ChangeCleanupRunner; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfigModule; @@ -63,6 +62,7 @@ import com.google.gerrit.server.schema.DataSourceModule; import com.google.gerrit.server.schema.DataSourceProvider; import com.google.gerrit.server.schema.DataSourceType; import com.google.gerrit.server.schema.DatabaseModule; +import com.google.gerrit.server.schema.H2AccountPatchReviewStore; import com.google.gerrit.server.schema.SchemaModule; import com.google.gerrit.server.schema.SchemaVersionCheck; import com.google.gerrit.server.securestore.SecureStoreClassName; @@ -298,7 +298,7 @@ public class WebAppInitializer extends GuiceServletContextListener final List modules = new ArrayList<>(); modules.add(new DropWizardMetricMaker.RestModule()); modules.add(new EventBroker.Module()); - modules.add(new AccountPatchReviewStoreImpl.Module()); + modules.add(new H2AccountPatchReviewStore.Module()); modules.add(cfgInjector.getInstance(GitRepositoryManagerModule.class)); modules.add(new ChangeHookApiListener.Module()); modules.add(new StreamEventsApiListener.Module());