opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-2.15' into stable-2.16

Browse Source 
* stable-2.15:
  Bazel: Enable Error-Prone by default
  Parallelize account migration in schema 146 migration
  Fix _nextStepHandle not being assigned a value when scrolling
  Elasticsearch: Remove redundant handling of 'ignore_unmapped' property
  Elasticsearch: Fix support for V6 versions earlier than 6.7.*
  ElasticVersionTest: Add missing assertions on V6_7

Change-Id: Ib655c541a28e3a9a2197465a58e1eff12123c6a5
This commit is contained in:
David Pursehouse
2019-05-28 11:51:47 +09:00
parent 229ee21afa 446c1f983e
commit 6d4cf6d026
9 changed files with 150 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.bazelrc
View File

@@ -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

1
java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
View File

@@ -294,7 +294,6 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
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);

1
java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
View File

@@ -175,7 +175,6 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
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);

15
java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java
View File

@@ -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) {

12
java/com/google/gerrit/elasticsearch/ElasticVersion.java
View File

@@ -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;

117
java/com/google/gerrit/server/schema/Schema_146.java
View File

@@ -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<Entry<Account.Id, Timestamp>> accounts = scanAccounts(db, ui).entrySet();
gc(ui);
Set<List<Entry<Account.Id, Timestamp>>> 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<Entry<Account.Id, Timestamp>> 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<Account.Id, Timestamp> e : scanAccounts(db).entrySet()) {
for (Map.Entry<Account.Id, Timestamp> 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<Account.Id, Timestamp> scanAccounts(ReviewDb db) throws SQLException {
private Map<Account.Id, Timestamp> 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<Account.Id, Timestamp> m = new HashMap<>();
while (rs.next()) {
m.put(new Account.Id(rs.getInt(1)), rs.getTimestamp(2));
}
size = m.size();
return m;
}
}

14
javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java
View File

@@ -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();
}
}

2
polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
View File

@@ -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.

11
tools/BUILD
View File

@@ -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/...",
],
)