From a6c058a26094bf169320d09e9fa1eff7faa5f432 Mon Sep 17 00:00:00 2001 From: Chris Hostetter Date: Mon, 3 Aug 2020 09:58:16 -0700 Subject: [PATCH] SOLR-14657: Improve error handling in IndexReader realted metrics that were causing scary ERROR logging if metrics were requested while Solr was in the process of closing/re-opening a new IndexReader --- solr/CHANGES.txt | 3 ++ .../apache/solr/search/SolrIndexSearcher.java | 29 +++++++++++++++---- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index b46ccfea886..1fc5a5f7200 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -156,6 +156,9 @@ Bug Fixes * SOLR-14516: Fix NPE in JSON response writer(wt=json) with /get when writing non-stored, non-indexed docvalue field from an uncommitted document (noble, Ishan Chattopadhyaya, Munendra S N) +* SOLR-14657: Improve error handling in IndexReader realted metrics that were causing scary ERROR logging + if metrics were requested while Solr was in the process of closing/re-opening a new IndexReader. (hossman) + Other Changes --------------------- diff --git a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java index 4cc97586118..3ebd43c845e 100644 --- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java +++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java @@ -35,7 +35,9 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; +import com.codahale.metrics.Gauge; import com.google.common.collect.Iterables; + import org.apache.lucene.document.Document; import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.ExitableDirectoryReader; @@ -51,6 +53,7 @@ import org.apache.lucene.index.Terms; import org.apache.lucene.index.TermsEnum; import org.apache.lucene.search.*; import org.apache.lucene.search.TotalHits.Relation; +import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.store.Directory; import org.apache.lucene.util.Bits; import org.apache.lucene.util.BytesRef; @@ -2274,12 +2277,12 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI parentContext.gauge(() -> warmupTime, true, "warmupTime", Category.SEARCHER.toString(), scope); parentContext.gauge(() -> registerTime, true, "registeredAt", Category.SEARCHER.toString(), scope); // reader stats - parentContext.gauge(() -> reader.numDocs(), true, "numDocs", Category.SEARCHER.toString(), scope); - parentContext.gauge(() -> reader.maxDoc(), true, "maxDoc", Category.SEARCHER.toString(), scope); - parentContext.gauge(() -> reader.maxDoc() - reader.numDocs(), true, "deletedDocs", Category.SEARCHER.toString(), scope); - parentContext.gauge(() -> reader.toString(), true, "reader", Category.SEARCHER.toString(), scope); - parentContext.gauge(() -> reader.directory().toString(), true, "readerDir", Category.SEARCHER.toString(), scope); - parentContext.gauge(() -> reader.getVersion(), true, "indexVersion", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge(-1, () -> reader.numDocs()), true, "numDocs", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge(-1, () -> reader.maxDoc()), true, "maxDoc", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge(-1, () -> reader.maxDoc() - reader.numDocs()), true, "deletedDocs", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge(-1, () -> reader.toString()), true, "reader", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge("", () -> reader.directory().toString()), true, "readerDir", Category.SEARCHER.toString(), scope); + parentContext.gauge(rgauge(-1, () -> reader.getVersion()), true, "indexVersion", Category.SEARCHER.toString(), scope); // size of the currently opened commit parentContext.gauge(() -> { try { @@ -2301,6 +2304,20 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI }), true, "statsCache", Category.CACHE.toString(), scope); } + /** + * wraps a gauge (related to an IndexReader) and swallows any {@link AlreadyClosedException} that + * might be thrown, returning the specified default in it's place. + */ + private Gauge rgauge(T closedDefault, Gauge g) { + return () -> { + try { + return g.getValue(); + } catch (AlreadyClosedException ignore) { + return closedDefault; + } + }; + } + private static class FilterImpl extends Filter { private final Filter topFilter; private final List weights;