diff --git a/.bazelrc b/.bazelrc index 4a89eed7c9..d6d4ce6cd3 100644 --- a/.bazelrc +++ b/.bazelrc @@ -3,6 +3,7 @@ build --repository_cache=~/.gerritcodereview/bazel-cache/repository build --experimental_strict_action_env build --action_env=PATH build --disk_cache=~/.gerritcodereview/bazel-cache/cas +build --java_toolchain //tools:error_prone_warnings_toolchain test --build_tests_only test --test_output=errors diff --git a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java index 95d49e5fac..8f18b61643 100644 --- a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java +++ b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java @@ -294,7 +294,6 @@ abstract class AbstractElasticIndex implements Index { protected JsonArray getSortArray(String idFieldName) { JsonObject properties = new JsonObject(); properties.addProperty(ORDER, "asc"); - client.adapter().setIgnoreUnmapped(properties); JsonArray sortArray = new JsonArray(); addNamedElement(idFieldName, properties, sortArray); diff --git a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java index 71fe434503..5a43ba8d1d 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java +++ b/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java @@ -175,7 +175,6 @@ class ElasticChangeIndex extends AbstractElasticIndex private JsonArray getSortArray() { JsonObject properties = new JsonObject(); properties.addProperty(ORDER, "desc"); - client.adapter().setIgnoreUnmapped(properties); JsonArray sortArray = new JsonArray(); addNamedElement(ChangeField.UPDATED.getName(), properties, sortArray); diff --git a/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java b/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java index e34644ed35..b0156781fc 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java +++ b/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java @@ -14,6 +14,8 @@ package com.google.gerrit.elasticsearch; +import static com.google.gerrit.elasticsearch.ElasticVersion.V6_7; + import com.google.gson.JsonObject; public class ElasticQueryAdapter { @@ -22,7 +24,6 @@ public class ElasticQueryAdapter { private static final String INCLUDE_TYPE = "include_type_name=true"; private static final String INDICES = "?allow_no_indices=false"; - private final boolean ignoreUnmapped; private final boolean useV5Type; private final boolean useV6Type; private final boolean omitType; @@ -37,24 +38,18 @@ public class ElasticQueryAdapter { private final String includeTypeNameParam; ElasticQueryAdapter(ElasticVersion version) { - this.ignoreUnmapped = false; this.useV5Type = !version.isV6OrLater(); this.useV6Type = version.isV6(); this.omitType = version.isV7OrLater(); this.versionDiscoveryUrl = version.isV6OrLater() ? "/%s*" : "/%s*/_aliases"; this.searchFilteringName = "_source"; - this.indicesExistParams = version.isV6() ? INDICES + "&" + INCLUDE_TYPE : INDICES; + this.indicesExistParams = + version.isAtLeastMinorVersion(V6_7) ? INDICES + "&" + INCLUDE_TYPE : INDICES; this.exactFieldType = "keyword"; this.stringFieldType = "text"; this.indexProperty = "true"; this.rawFieldsKey = "_source"; - this.includeTypeNameParam = version.isV6() ? "?" + INCLUDE_TYPE : ""; - } - - void setIgnoreUnmapped(JsonObject properties) { - if (ignoreUnmapped) { - properties.addProperty("ignore_unmapped", true); - } + this.includeTypeNameParam = version.isAtLeastMinorVersion(V6_7) ? "?" + INCLUDE_TYPE : ""; } public void setType(JsonObject properties, String type) { diff --git a/java/com/google/gerrit/elasticsearch/ElasticVersion.java b/java/com/google/gerrit/elasticsearch/ElasticVersion.java index 98e1f7d07c..6be41c8109 100644 --- a/java/com/google/gerrit/elasticsearch/ElasticVersion.java +++ b/java/com/google/gerrit/elasticsearch/ElasticVersion.java @@ -71,14 +71,22 @@ public enum ElasticVersion { return isAtLeastVersion(7); } - private boolean isAtLeastVersion(int v) { - return getMajor() >= v; + private boolean isAtLeastVersion(int major) { + return getMajor() >= major; + } + + public boolean isAtLeastMinorVersion(ElasticVersion version) { + return getMajor().equals(version.getMajor()) && getMinor() >= version.getMinor(); } private Integer getMajor() { return Integer.valueOf(version.split("\\.")[0]); } + private Integer getMinor() { + return Integer.valueOf(version.split("\\.")[1]); + } + @Override public String toString() { return version; diff --git a/java/com/google/gerrit/server/schema/Schema_146.java b/java/com/google/gerrit/server/schema/Schema_146.java index 9ca61d9f4d..64e9123c95 100644 --- a/java/com/google/gerrit/server/schema/Schema_146.java +++ b/java/com/google/gerrit/server/schema/Schema_146.java @@ -14,6 +14,9 @@ package com.google.gerrit.server.schema; +import com.google.common.base.Stopwatch; +import com.google.common.collect.Iterables; +import com.google.common.collect.Sets; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -24,21 +27,36 @@ import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; import java.io.IOException; +import java.io.UncheckedIOException; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; import java.sql.Timestamp; +import java.text.ParseException; +import java.util.Date; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.ReentrantLock; +import org.eclipse.jgit.internal.storage.file.FileRepository; +import org.eclipse.jgit.internal.storage.file.GC; import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.ProgressMonitor; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.RefUpdate.Result; import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.TextProgressMonitor; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevSort; import org.eclipse.jgit.revwalk.RevWalk; @@ -60,6 +78,10 @@ public class Schema_146 extends SchemaVersion { private final GitRepositoryManager repoManager; private final AllUsersName allUsersName; private final PersonIdent serverIdent; + private AtomicInteger i = new AtomicInteger(); + private Stopwatch sw = Stopwatch.createStarted(); + ReentrantLock gcLock = new ReentrantLock(); + private int size; @Inject Schema_146( @@ -75,13 +97,41 @@ public class Schema_146 extends SchemaVersion { @Override protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException { + ui.message("Migrating accounts"); + Set> accounts = scanAccounts(db, ui).entrySet(); + gc(ui); + Set>> batches = + Sets.newHashSet(Iterables.partition(accounts, 500)); + ExecutorService pool = createExecutor(ui); + try { + batches.stream().forEach(batch -> pool.submit(() -> processBatch(batch, ui))); + pool.shutdown(); + pool.awaitTermination(Long.MAX_VALUE, TimeUnit.DAYS); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + ui.message( + String.format("... (%.3f s) Migrated all %d accounts to schema 146", elapsed(), i.get())); + } + + private ExecutorService createExecutor(UpdateUI ui) { + int threads; + try { + threads = Integer.parseInt(System.getProperty("threadcount")); + } catch (NumberFormatException e) { + threads = Runtime.getRuntime().availableProcessors(); + } + ui.message(String.format("... using %d threads ...", threads)); + return Executors.newFixedThreadPool(threads); + } + + private void processBatch(List> batch, UpdateUI ui) { try (Repository repo = repoManager.openRepository(allUsersName); RevWalk rw = new RevWalk(repo); ObjectInserter oi = repo.newObjectInserter()) { ObjectId emptyTree = emptyTree(oi); - int i = 0; - for (Map.Entry e : scanAccounts(db).entrySet()) { + for (Map.Entry e : batch) { String refName = RefNames.refsUsers(e.getKey()); Ref ref = repo.exactRef(refName); if (ref != null) { @@ -89,14 +139,63 @@ public class Schema_146 extends SchemaVersion { } else { createUserBranch(repo, oi, emptyTree, e.getKey(), e.getValue()); } - i++; - if (i % 100 == 0) { - ui.message(String.format("... migrated %d users", i)); + int count = i.incrementAndGet(); + showProgress(ui, count); + if (count % 1000 == 0) { + gc(repo, true, ui); } } - ui.message(String.format("Migrated all %d users to schema 146", i)); } catch (IOException e) { - throw new OrmException("Failed to rewrite user branches.", e); + throw new UncheckedIOException(e); + } + } + + private double elapsed() { + return sw.elapsed(TimeUnit.MILLISECONDS) / 1000d; + } + + private void showProgress(UpdateUI ui, int count) { + if (count % 100 == 0) { + ui.message( + String.format( + "... (%.3f s) migrated %d%% (%d/%d) accounts", + elapsed(), Math.round(100.0 * count / size), count, size)); + } + } + + private void gc(UpdateUI ui) { + try (Repository repo = repoManager.openRepository(allUsersName)) { + gc(repo, false, ui); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + private void gc(Repository repo, boolean refsOnly, UpdateUI ui) { + if (repo instanceof FileRepository && gcLock.tryLock()) { + ProgressMonitor pm = null; + try { + pm = new TextProgressMonitor(); + FileRepository r = (FileRepository) repo; + GC gc = new GC(r); + gc.setProgressMonitor(pm); + pm.beginTask("gc", ProgressMonitor.UNKNOWN); + if (refsOnly) { + ui.message(String.format("... (%.3f s) pack refs", elapsed())); + gc.packRefs(); + } else { + ui.message(String.format("... (%.3f s) gc --prune=now", elapsed())); + gc.setExpire(new Date()); + gc.gc(); + } + } catch (IOException | ParseException e) { + throw new RuntimeException(e); + } finally { + gcLock.unlock(); + if (pm != null) { + pm.endTask(); + } + } } } @@ -189,13 +288,15 @@ public class Schema_146 extends SchemaVersion { return oi.insert(Constants.OBJ_TREE, new byte[] {}); } - private Map scanAccounts(ReviewDb db) throws SQLException { + private Map scanAccounts(ReviewDb db, UpdateUI ui) throws SQLException { + ui.message(String.format("... (%.3f s) scan accounts", elapsed())); try (Statement stmt = newStatement(db); ResultSet rs = stmt.executeQuery("SELECT account_id, registered_on FROM accounts")) { HashMap m = new HashMap<>(); while (rs.next()) { m.put(new Account.Id(rs.getInt(1)), rs.getTimestamp(2)); } + size = m.size(); return m; } } diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java index b2b15aec60..cb0f555fd2 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java @@ -66,9 +66,22 @@ public class ElasticVersionTest { assertThat(ElasticVersion.V6_4.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V6_5.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V6_6.isV6OrLater()).isTrue(); + assertThat(ElasticVersion.V6_7.isV6OrLater()).isTrue(); assertThat(ElasticVersion.V7_0.isV6OrLater()).isTrue(); } + @Test + public void atLeastMinorVersion() throws Exception { + assertThat(ElasticVersion.V5_6.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); + assertThat(ElasticVersion.V6_2.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); + assertThat(ElasticVersion.V6_3.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); + assertThat(ElasticVersion.V6_4.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); + assertThat(ElasticVersion.V6_5.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); + assertThat(ElasticVersion.V6_6.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); + assertThat(ElasticVersion.V6_7.isAtLeastMinorVersion(ElasticVersion.V6_7)).isTrue(); + assertThat(ElasticVersion.V7_0.isAtLeastMinorVersion(ElasticVersion.V6_7)).isFalse(); + } + @Test public void version7() throws Exception { assertThat(ElasticVersion.V5_6.isV7OrLater()).isFalse(); @@ -77,6 +90,7 @@ public class ElasticVersionTest { assertThat(ElasticVersion.V6_4.isV7OrLater()).isFalse(); assertThat(ElasticVersion.V6_5.isV7OrLater()).isFalse(); assertThat(ElasticVersion.V6_6.isV7OrLater()).isFalse(); + assertThat(ElasticVersion.V6_7.isV7OrLater()).isFalse(); assertThat(ElasticVersion.V7_0.isV7OrLater()).isTrue(); } } diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js index dead8d75f2..0ed2db2997 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js @@ -126,7 +126,7 @@ let currentBatch = 0; const nextStep = () => { if (this._isScrolling) { - this.async(nextStep, 100); + this._nextStepHandle = this.async(nextStep, 100); return; } // If we are done, resolve the promise. diff --git a/tools/BUILD b/tools/BUILD index d4ab2a8c02..d25a26eb76 100644 --- a/tools/BUILD +++ b/tools/BUILD @@ -23,8 +23,10 @@ default_java_toolchain( visibility = ["//visibility:public"], ) -# This EP warnings list is based on: +# Error Prone errors enabled by default; see ../.bazelrc for how this is +# enabled. This warnings list is originally based on: # https://github.com/bazelbuild/BUILD_file_generator/blob/master/tools/bazel_defs/java.bzl +# However, feel free to add any additional errors. Thus far they have all been pretty useful. java_package_configuration( name = "error_prone", javacopts = [ @@ -96,5 +98,12 @@ package_group( packages = [ "//java/...", "//javatests/...", + "//plugins/codemirror-editor/...", + "//plugins/commit-message-length-validator/...", + "//plugins/download-commands/...", + "//plugins/hooks/...", + "//plugins/replication/...", + "//plugins/reviewnotes/...", + "//plugins/singleusergroup/...", ], )