Merge "Improve Jetty thread pool metrics" into stable-2.16
diff --git a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
index ebec9c5..d6daae1 100644
--- a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
+++ b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
@@ -72,6 +72,7 @@
import com.google.gerrit.server.notedb.RepoSequence;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException;
import com.google.gerrit.server.project.NoSuchChangeException;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.update.ChainedReceiveCommands;
import com.google.gerrit.server.update.RefUpdateUtil;
import com.google.gerrit.server.util.ManualRequestContext;
@@ -164,6 +165,7 @@
private final MutableNotesMigration globalNotesMigration;
private final PrimaryStorageMigrator primaryStorageMigrator;
private final DynamicSet<NotesMigrationStateListener> listeners;
+ private final ProjectCache projectCache;
private int threads;
private ImmutableList<Project.NameKey> projects = ImmutableList.of();
@@ -193,7 +195,8 @@
WorkQueue workQueue,
MutableNotesMigration globalNotesMigration,
PrimaryStorageMigrator primaryStorageMigrator,
- DynamicSet<NotesMigrationStateListener> listeners) {
+ DynamicSet<NotesMigrationStateListener> listeners,
+ ProjectCache projectCache) {
// Reload gerrit.config/notedb.config on each migrator invocation, in case a previous
// migration in the same process modified the on-disk contents. This ensures the defaults for
// trial/autoMigrate get set correctly below.
@@ -213,6 +216,7 @@
this.globalNotesMigration = globalNotesMigration;
this.primaryStorageMigrator = primaryStorageMigrator;
this.listeners = listeners;
+ this.projectCache = projectCache;
this.trial = getTrialMode(cfg);
this.autoMigrate = getAutoMigrate(cfg);
}
@@ -400,6 +404,7 @@
changes,
progressOut,
stopAtState,
+ projectCache,
trial,
forceRebuild,
sequenceGap >= 0 ? sequenceGap : Sequences.getChangeSequenceGap(cfg),
@@ -429,6 +434,7 @@
private final ImmutableList<Change.Id> changes;
private final OutputStream progressOut;
private final NotesMigrationState stopAtState;
+ private final ProjectCache projectCache;
private final boolean trial;
private final boolean forceRebuild;
private final int sequenceGap;
@@ -455,6 +461,7 @@
ImmutableList<Change.Id> changes,
OutputStream progressOut,
NotesMigrationState stopAtState,
+ ProjectCache projectCache,
boolean trial,
boolean forceRebuild,
int sequenceGap,
@@ -489,6 +496,7 @@
this.changes = changes;
this.progressOut = progressOut;
this.stopAtState = stopAtState;
+ this.projectCache = projectCache;
this.trial = trial;
this.forceRebuild = forceRebuild;
this.sequenceGap = sequenceGap;
@@ -702,11 +710,13 @@
* of the NoteDb migration code, which is too risky to attempt in the stable branch where this bug
* had to be fixed.
*
- * <p>As of this writing, the only case where this happens is when a change has no patch sets.
+ * <p>As of this writing, there are only two cases where this happens: when a change has no patch
+ * sets, or the project doesn't exist.
*/
- private static boolean canSkipPrimaryStorageMigration(ReviewDb db, Change.Id id) {
+ private boolean canSkipPrimaryStorageMigration(ReviewDb db, Change.Id id) {
try {
- return Iterables.isEmpty(unwrapDb(db).patchSets().byChange(id));
+ return Iterables.isEmpty(unwrapDb(db).patchSets().byChange(id))
+ || projectCache.get(unwrapDb(db).changes().get(id).getProject()) == null;
} catch (Exception e) {
logger.atSevere().withCause(e).log(
"Error checking if change %s can be skipped, assuming no", id);
diff --git a/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java b/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
index c249973..b9ed0f3 100644
--- a/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/notedb/OnlineNoteDbMigrationIT.java
@@ -35,6 +35,8 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.io.MoreFiles;
+import com.google.common.io.RecursiveDeleteOption;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.NoHttpd;
@@ -84,6 +86,7 @@
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.RepositoryCache.FileKey;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
import org.junit.After;
@@ -511,6 +514,42 @@
}
@Test
+ public void fullMigrationOneChangeWithNoProject() throws Exception {
+ PushOneCommit.Result r1 = createChange();
+ Change.Id id1 = r1.getChange().getId();
+
+ Project.NameKey p2 = createProject("project2");
+ TestRepository<?> tr2 = cloneProject(p2, admin);
+ PushOneCommit.Result r2 = pushFactory.create(db, admin.getIdent(), tr2).to("refs/for/master");
+ Change.Id id2 = r2.getChange().getId();
+
+ // TODO(davido): Find an easier way to wipe out a repository from the file system.
+ MoreFiles.deleteRecursively(
+ FileKey.lenient(
+ sitePaths
+ .resolve(cfg.getString("gerrit", null, "basePath"))
+ .resolve(p2.get())
+ .toFile(),
+ FS.DETECTED)
+ .getFile()
+ .toPath(),
+ RecursiveDeleteOption.ALLOW_INSECURE);
+
+ migrate(b -> b);
+ assertNotesMigrationState(NOTE_DB, false, false);
+
+ try (ReviewDb db = schemaFactory.open();
+ Repository repo = repoManager.openRepository(project)) {
+ assertThat(repo.exactRef(RefNames.changeMetaRef(id1))).isNotNull();
+ assertThat(db.changes().get(id1).getNoteDbState()).isEqualTo(NOTE_DB_PRIMARY_STATE);
+ }
+
+ // A change without project is so corrupt that it is completely skipped by the migration
+ // process.
+ assertThat(db.changes().get(id2).getNoteDbState()).isNull();
+ }
+
+ @Test
public void fullMigrationMissingPatchSetRefs() throws Exception {
PushOneCommit.Result r = createChange();
Change.Id id = r.getChange().getId();
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
index 6d413b7..19839a8 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/gr-linked-text_test.html
@@ -50,6 +50,10 @@
match: '([Bb]ug|[Ii]ssue)\\s*#?(\\d+)',
link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
},
+ prefixsameinlinkandpattern: {
+ match: '([Hh][Tt][Tt][Pp]example)\\s*#?(\\d+)',
+ link: 'https://bugs.chromium.org/p/gerrit/issues/detail?id=$2',
+ },
changeid: {
match: '(I[0-9a-f]{8,40})',
link: '#/q/$1',
@@ -116,6 +120,18 @@
assert.equal(linkEl.textContent, 'Bug 3650');
});
+ test('Pattern with same prefix as link was correctly parsed', () => {
+ // Pattern starts with the same prefix (`http`) as the url.
+ element.content = 'httpexample 3650';
+
+ assert.equal(element.$.output.childNodes.length, 1);
+ const linkEl = element.$.output.childNodes[0];
+ const url = 'https://bugs.chromium.org/p/gerrit/issues/detail?id=3650';
+ assert.equal(linkEl.target, '_blank');
+ assert.equal(linkEl.href, url);
+ assert.equal(linkEl.textContent, 'httpexample 3650');
+ });
+
test('Change-Id pattern was parsed and linked', () => {
// "Change-Id:" pattern.
const changeID = 'I11d6a37f5e9b5df0486f6c922d8836dfa780e03e';
diff --git a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
index 23a71f9..027c632 100644
--- a/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
+++ b/polygerrit-ui/app/elements/shared/gr-linked-text/link-text-parser.js
@@ -312,14 +312,15 @@
let result = match[0].replace(pattern,
patterns[p].html || patterns[p].link);
- let i;
- // Skip portion of replacement string that is equal to original.
- for (i = 0; i < result.length; i++) {
- if (result[i] !== match[0][i]) { break; }
- }
- result = result.slice(i);
-
if (patterns[p].html) {
+ let i;
+ // Skip portion of replacement string that is equal to original to
+ // allow overlapping patterns.
+ for (i = 0; i < result.length; i++) {
+ if (result[i] !== match[0][i]) { break; }
+ }
+ result = result.slice(i);
+
this.addHTML(
result,
susbtrIndex + match.index + i,
@@ -329,8 +330,8 @@
this.addLink(
match[0],
result,
- susbtrIndex + match.index + i,
- match[0].length - i,
+ susbtrIndex + match.index,
+ match[0].length,
outputArray);
} else {
throw Error('linkconfig entry ' + p +
