From 19ef754888e1545ec9a0095fd684d7e01f78ba52 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Sun, 19 Jun 2016 21:39:43 -0700 Subject: [PATCH 1/4] Run all Lucene queries on background threads Backport of d2d2d3907f199fbb722ee0d71da5daaa896142ed to stable-2.12. The NIOFSDirectory type does not handle being interrupted well, as an interrupt delivered by an SSH connection closing will close the file handles used to read the index. Isolate the Lucene reader from the SSH threads by always running a Lucene query on the interactive index thread pool. This may reduce latency for user dashboards on a multi-core system as each query for the different sections can now run on separate threads and return results when ready. Bug: Issue 4400 Change-Id: I9fd97298d000e2b32371b9e0c454f7db3ef867ab --- .../gerrit/lucene/LuceneChangeIndex.java | 77 ++++++++++++++----- .../server/query/change/QueryProcessor.java | 8 +- 2 files changed, 64 insertions(+), 21 deletions(-) diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java index d703bed0cd..e65e32cf45 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -23,6 +23,7 @@ import static com.google.gerrit.server.index.IndexRewriter.OPEN_STATUSES; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.concurrent.TimeUnit.MINUTES; +import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; @@ -58,6 +59,7 @@ import com.google.gerrit.server.query.change.LegacyChangeIdPredicate; import com.google.gerrit.server.query.change.QueryOptions; import com.google.gwtorm.protobuf.ProtobufCodec; import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.OrmRuntimeException; import com.google.gwtorm.server.ResultSet; import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; @@ -110,7 +112,9 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; /** * Secondary index implementation using Apache Lucene. @@ -426,6 +430,20 @@ public class LuceneChangeIndex implements ChangeIndex { @Override public ResultSet read() throws OrmException { + if (Thread.interrupted()) { + Thread.currentThread().interrupt(); + throw new OrmException("interupted"); + } + return new ChangeDataResults( + executor.submit(new Callable>() { + @Override + public List call() throws OrmException { + return doRead(); + } + })); + } + + private List doRead() throws OrmException { IndexSearcher[] searchers = new IndexSearcher[indexes.size()]; try { int realLimit = opts.start() + opts.limit(); @@ -436,31 +454,14 @@ public class LuceneChangeIndex implements ChangeIndex { } TopDocs docs = TopDocs.merge(sort, realLimit, hits); - List result = + List result = Lists.newArrayListWithCapacity(docs.scoreDocs.length); for (int i = opts.start(); i < docs.scoreDocs.length; i++) { ScoreDoc sd = docs.scoreDocs[i]; Document doc = searchers[sd.shardIndex].doc(sd.doc, FIELDS); - result.add(toChangeData(doc)); + result.add(doc); } - - final List r = Collections.unmodifiableList(result); - return new ResultSet() { - @Override - public Iterator iterator() { - return r.iterator(); - } - - @Override - public List toList() { - return r; - } - - @Override - public void close() { - // Do nothing. - } - }; + return result; } catch (IOException e) { throw new OrmException(e); } finally { @@ -477,6 +478,42 @@ public class LuceneChangeIndex implements ChangeIndex { } } + private class ChangeDataResults implements ResultSet { + private final Future> future; + + ChangeDataResults(Future> future) { + this.future = future; + } + + @Override + public Iterator iterator() { + return toList().iterator(); + } + + @Override + public List toList() { + try { + List docs = future.get(); + List result = new ArrayList<>(docs.size()); + String idFieldName = ChangeField.LEGACY_ID.getName(); + for (Document doc : docs) { + result.add(toChangeData(doc)); + } + return result; + } catch (InterruptedException e) { + close(); + throw new OrmRuntimeException(e); + } catch (ExecutionException e) { + Throwables.propagateIfPossible(e.getCause()); + throw new OrmRuntimeException(e.getCause()); + } + } + + @Override + public void close() { + future.cancel(false /* do not interrupt Lucene */); + } + } private ChangeData toChangeData(Document doc) { BytesRef cb = doc.getBinaryValue(CHANGE_FIELD); if (cb == null) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java index a2c8b81981..870ca040bc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java @@ -29,6 +29,7 @@ import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; import com.google.gwtorm.server.OrmException; +import com.google.gwtorm.server.OrmRuntimeException; import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; import com.google.inject.Provider; @@ -100,7 +101,12 @@ public class QueryProcessor { */ public List queryChanges(List> queries) throws OrmException, QueryParseException { - return queryChanges(null, queries); + try { + return queryChanges(null, queries); + } catch (OrmRuntimeException e) { + throw new OrmException(e.getMessage(), e); + } + } static { From f68f65edcbca75c98700e6b79bebdf57a2171dfc Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 16 Aug 2016 15:37:53 +0900 Subject: [PATCH 2/4] Update 2.12.4 release notes Change-Id: Ib9ca21ddb060a9a0bd5d9000c824821bff031ebf --- ReleaseNotes/ReleaseNotes-2.12.4.txt | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/ReleaseNotes/ReleaseNotes-2.12.4.txt b/ReleaseNotes/ReleaseNotes-2.12.4.txt index ce8651c462..21ee41de7e 100644 --- a/ReleaseNotes/ReleaseNotes-2.12.4.txt +++ b/ReleaseNotes/ReleaseNotes-2.12.4.txt @@ -55,6 +55,19 @@ according to the comments in the issue. == Bug Fixes +* link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4400[Issue 4400]: +Fix `AlreadyClosedException` in Lucene index. ++ +If a Lucene indexing thread was interrupted by an SSH connection being +closed, this would also close file handles being used to read the index. ++ +Lucene queries are now executed on background threads to isolate them +from SSH threads. ++ +This may also reduce latency for user dashboards on a multi-core system as +each query for the different sections can now run on separate threads and +return results when ready. + * link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4249[Issue 4249]: Fix 'Duplicate stages not allowed' error during indexing. @@ -102,3 +115,14 @@ commit when submitting a merge commit. * link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4332[Issue 4332]: Allow `local` as a valid TLD for outgoing emails. + +* Bypass hostname verification when `sendemail.sslVerify` is disabled. + +* link:https://bugs.chromium.org/p/gerrit/issues/detail?id=4398[Issue 4398]: +Replication: Consider ref visibility when scheduling replication. ++ +It was possible for refs to be replicated to remotes despite not being +visible to groups mentioned in the `authGroup` setting. + +* link::https://bugs.chromium.org/p/gerrit/issues/detail?id=4036[Issue 4036]: +Fix hanging query when using `is:watched` without authentication. From a9a81339e8db569c19a245ef2afe77c76714c4fd Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 11 Aug 2016 14:43:56 +0900 Subject: [PATCH 3/4] Set version to 2.12.4 Change-Id: I346458961062e97191aced3395bfd443565873e1 --- Documentation/dev-plugins.txt | 2 +- VERSION | 2 +- gerrit-acceptance-framework/pom.xml | 2 +- gerrit-extension-api/pom.xml | 2 +- gerrit-plugin-api/pom.xml | 2 +- gerrit-plugin-archetype/pom.xml | 2 +- gerrit-plugin-gwt-archetype/pom.xml | 2 +- gerrit-plugin-gwtui/pom.xml | 2 +- gerrit-plugin-js-archetype/pom.xml | 2 +- gerrit-war/pom.xml | 2 +- 10 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt index c1a5dd6908..470c6b6c5a 100644 --- a/Documentation/dev-plugins.txt +++ b/Documentation/dev-plugins.txt @@ -36,7 +36,7 @@ plugin project. ---- mvn archetype:generate -DarchetypeGroupId=com.google.gerrit \ -DarchetypeArtifactId=gerrit-plugin-archetype \ - -DarchetypeVersion=2.12.3 \ + -DarchetypeVersion=2.12.4 \ -DgroupId=com.googlesource.gerrit.plugins.testplugin \ -DartifactId=testplugin ---- diff --git a/VERSION b/VERSION index 0138f8afe6..f005129eed 100644 --- a/VERSION +++ b/VERSION @@ -2,4 +2,4 @@ # Used by :api_install and :api_deploy targets # when talking to the destination repository. # -GERRIT_VERSION = '2.12.3' +GERRIT_VERSION = '2.12.4' diff --git a/gerrit-acceptance-framework/pom.xml b/gerrit-acceptance-framework/pom.xml index 6841ace572..50ef5c1894 100644 --- a/gerrit-acceptance-framework/pom.xml +++ b/gerrit-acceptance-framework/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-acceptance-framework - 2.12.3 + 2.12.4 jar Gerrit Code Review - Acceptance Test Framework API for Gerrit Plugins diff --git a/gerrit-extension-api/pom.xml b/gerrit-extension-api/pom.xml index 86c5bb7a72..ff88248611 100644 --- a/gerrit-extension-api/pom.xml +++ b/gerrit-extension-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-extension-api - 2.12.3 + 2.12.4 jar Gerrit Code Review - Extension API API for Gerrit Extensions diff --git a/gerrit-plugin-api/pom.xml b/gerrit-plugin-api/pom.xml index fa921fd494..671d0e2ff4 100644 --- a/gerrit-plugin-api/pom.xml +++ b/gerrit-plugin-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-api - 2.12.3 + 2.12.4 jar Gerrit Code Review - Plugin API API for Gerrit Plugins diff --git a/gerrit-plugin-archetype/pom.xml b/gerrit-plugin-archetype/pom.xml index dc3c272151..d39a9cae46 100644 --- a/gerrit-plugin-archetype/pom.xml +++ b/gerrit-plugin-archetype/pom.xml @@ -20,7 +20,7 @@ limitations under the License. com.google.gerrit gerrit-plugin-archetype - 2.12.3 + 2.12.4 Gerrit Code Review - Plugin Archetype Maven Archetype for Gerrit Plugins https://www.gerritcodereview.com/ diff --git a/gerrit-plugin-gwt-archetype/pom.xml b/gerrit-plugin-gwt-archetype/pom.xml index bbd461919c..658aaded47 100644 --- a/gerrit-plugin-gwt-archetype/pom.xml +++ b/gerrit-plugin-gwt-archetype/pom.xml @@ -20,7 +20,7 @@ limitations under the License. com.google.gerrit gerrit-plugin-gwt-archetype - 2.12.3 + 2.12.4 Gerrit Code Review - Web UI GWT Plugin Archetype Maven Archetype for Gerrit Web UI GWT Plugins https://www.gerritcodereview.com/ diff --git a/gerrit-plugin-gwtui/pom.xml b/gerrit-plugin-gwtui/pom.xml index 399dd99edd..9da7d9d670 100644 --- a/gerrit-plugin-gwtui/pom.xml +++ b/gerrit-plugin-gwtui/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-gwtui - 2.12.3 + 2.12.4 jar Gerrit Code Review - Plugin GWT UI Common Classes for Gerrit GWT UI Plugins diff --git a/gerrit-plugin-js-archetype/pom.xml b/gerrit-plugin-js-archetype/pom.xml index 4c022ffafc..30ad2fc46b 100644 --- a/gerrit-plugin-js-archetype/pom.xml +++ b/gerrit-plugin-js-archetype/pom.xml @@ -20,7 +20,7 @@ limitations under the License. com.google.gerrit gerrit-plugin-js-archetype - 2.12.3 + 2.12.4 Gerrit Code Review - Web UI JavaScript Plugin Archetype Maven Archetype for Gerrit Web UI JavaScript Plugins https://www.gerritcodereview.com/ diff --git a/gerrit-war/pom.xml b/gerrit-war/pom.xml index 049c274cba..8a1445e4ed 100644 --- a/gerrit-war/pom.xml +++ b/gerrit-war/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-war - 2.12.3 + 2.12.4 war Gerrit Code Review - WAR Gerrit WAR From 87f6de7921ab919a516eb716e8ed809ed609c633 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 24 Aug 2016 09:22:18 +0900 Subject: [PATCH 4/4] LuceneChangeIndex: Remove unused variable Change-Id: Icb1f5642ee1f94a731f8b41622dd566c5f119f90 --- .../main/java/com/google/gerrit/lucene/LuceneChangeIndex.java | 1 - 1 file changed, 1 deletion(-) diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java index e65e32cf45..fe37362d09 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -495,7 +495,6 @@ public class LuceneChangeIndex implements ChangeIndex { try { List docs = future.get(); List result = new ArrayList<>(docs.size()); - String idFieldName = ChangeField.LEGACY_ID.getName(); for (Document doc : docs) { result.add(toChangeData(doc)); }