SOLR-9330: Fix AlreadyClosedException on admin/mbeans?stats=true

solr/CHANGES.txt

@@ -117,6 +117,7 @@ Bug Fixes
 
 * SOLR-9542: Kerberos delegation tokens requires Jackson library (Ishan Chattopadhyaya via noble)
 
+* SOLR-9330: Fix AlreadyClosedException on admin/mbeans?stats=true (Mikhail Khludnev) 
 
 Optimizations
 ----------------------

solr/core/src/java/org/apache/solr/core/SolrCore.java

@@ -2077,7 +2077,6 @@ public final class SolrCore implements SolrInfoMBean, Closeable {
       if (_searcher != null) {
         _searcher.decref();   // dec refcount for this._searcher
         _searcher = null; // isClosed() does check this
-        infoRegistry.remove("currentSearcher");
       }
     }
   }

solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java

@@ -96,6 +96,7 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   public static final String STATS_SOURCE = "org.apache.solr.stats_source";
+  public static final String STATISTICS_KEY = "searcher";
   // These should *only* be used for debugging or monitoring purposes
   public static final AtomicLong numOpens = new AtomicLong();
   public static final AtomicLong numCloses = new AtomicLong();
@@ -153,6 +154,8 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
 
   private final Map<Long, IndexFingerprint> maxVersionFingerprintCache = new ConcurrentHashMap<>();
 
+  private final NamedList<Object> readerStats;
+
   private static DirectoryReader getReader(SolrCore core, SolrIndexConfig config, DirectoryFactory directoryFactory,
       String path) throws IOException {
     final Directory dir = directoryFactory.get(path, DirContext.DEFAULT, config.lockType);
@@ -338,6 +341,7 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
     // We already have our own filter cache
     setQueryCache(null);
 
+    readerStats = snapStatistics(reader);
     // do this at the end since an exception in the constructor means we won't close
     numOpens.incrementAndGet();
   }
@@ -423,7 +427,7 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
   public void register() {
     final Map<String,SolrInfoMBean> infoRegistry = core.getInfoRegistry();
     // register self
-    infoRegistry.put("searcher", this);
+    infoRegistry.put(STATISTICS_KEY, this);
     infoRegistry.put(name, this);
     for (SolrCache cache : cacheList) {
       cache.setState(SolrCache.State.LIVE);
@@ -2449,15 +2453,23 @@ public class SolrIndexSearcher extends IndexSearcher implements Closeable, SolrI
     final NamedList<Object> lst = new SimpleOrderedMap<>();
     lst.add("searcherName", name);
     lst.add("caching", cachingEnabled);
+
+    lst.addAll(readerStats);
+
+    lst.add("openedAt", openTime);
+    if (registerTime != null) lst.add("registeredAt", registerTime);
+    lst.add("warmupTime", warmupTime);
+    return lst;
+  }
+
+  static private NamedList<Object> snapStatistics(DirectoryReader reader) {
+    final NamedList<Object> lst = new SimpleOrderedMap<>();
     lst.add("numDocs", reader.numDocs());
     lst.add("maxDoc", reader.maxDoc());
     lst.add("deletedDocs", reader.maxDoc() - reader.numDocs());
     lst.add("reader", reader.toString());
     lst.add("readerDir", reader.directory());
     lst.add("indexVersion", reader.getVersion());
-    lst.add("openedAt", openTime);
-    if (registerTime != null) lst.add("registeredAt", registerTime);
-    lst.add("warmupTime", warmupTime);
     return lst;
   }
 

solr/core/src/test/org/apache/solr/handler/admin/StatsReloadRaceTest.java

@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.admin;
+
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.params.CoreAdminParams;
+import org.apache.solr.common.params.CoreAdminParams.CoreAdminAction;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.response.SolrQueryResponse;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class StatsReloadRaceTest extends SolrTestCaseJ4 {
+
+  // to support many times repeating
+  static AtomicInteger taskNum = new AtomicInteger();
+
+  @BeforeClass
+  public static void beforeTests() throws Exception {
+    initCore("solrconfig.xml", "schema.xml");
+
+    XmlDoc docs = new XmlDoc();
+    for (int i = 0; i < atLeast(10); i++) {
+      docs.xml += doc("id", "" + i,
+          "name_s", "" + i);
+    }
+    assertU(add(docs));
+    assertU(commit());
+  }
+
+  @Test
+  public void testParallelReloadAndStats() throws Exception {
+
+    for (int i = 0; i < atLeast(2); i++) {
+
+      int asyncId = taskNum.incrementAndGet();
+
+      SolrQueryResponse rsp = new SolrQueryResponse();
+      h.getCoreContainer().getMultiCoreHandler().handleRequest(req(
+          CommonParams.QT, "/admin/cores",
+          CoreAdminParams.ACTION,
+          CoreAdminParams.CoreAdminAction.RELOAD.toString(),
+          CoreAdminParams.CORE, DEFAULT_TEST_CORENAME,
+          "async", "" + asyncId), new SolrQueryResponse());
+
+      boolean isCompleted;
+      do {
+        String stats = h.query(req(
+            CommonParams.QT, "/admin/mbeans",
+            "stats", "true"));
+
+        NamedList<NamedList<Object>> actualStats = SolrInfoMBeanHandler.fromXML(stats).get("CORE");
+        
+        for (Map.Entry<String, NamedList<Object>> tuple : actualStats) {
+          if (tuple.getKey().contains("earcher")) { // catches "searcher" and "Searcher@345345 blah"
+            NamedList<Object> searcherStats = tuple.getValue();
+            @SuppressWarnings("unchecked")
+            NamedList<Object> statsList = (NamedList<Object>)searcherStats.get("stats");
+            assertEquals("expect to have exactly one indexVersion at "+statsList, 1, statsList.getAll("indexVersion").size());
+            assertTrue(statsList.get("indexVersion") instanceof Long); 
+          }
+        }
+
+        h.getCoreContainer().getMultiCoreHandler().handleRequest(req(
+            CoreAdminParams.ACTION,
+            CoreAdminParams.CoreAdminAction.REQUESTSTATUS.toString(),
+            CoreAdminParams.REQUESTID, "" + asyncId), rsp);
+        
+        @SuppressWarnings("unchecked")
+        List<Object> statusLog = rsp.getValues().getAll(CoreAdminAction.STATUS.name());
+
+        assertFalse("expect status check w/o error, got:" + statusLog,
+                                  statusLog.contains(CoreAdminHandler.FAILED));
+
+        isCompleted = statusLog.contains(CoreAdminHandler.COMPLETED);
+      } while (!isCompleted);
+    }
+  }
+
+}