You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2020/09/02 12:47:23 UTC

[lucene-solr] 03/03: @699 Some review tweaks.

This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a commit to branch reference_impl
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 4c7a5c22c3b873a292b294e1304336077791a9c7
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Wed Sep 2 07:06:10 2020 -0500

    @699 Some review tweaks.
---
 solr/core/src/java/org/apache/solr/api/ApiBag.java |  2 +-
 .../src/java/org/apache/solr/core/SolrCore.java    | 30 +++++++++++-----------
 .../solr/core/CachingDirectoryFactoryTest.java     |  2 +-
 .../java/org/apache/solr/common/util/PathTrie.java | 27 ++++++++++---------
 4 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/api/ApiBag.java b/solr/core/src/java/org/apache/solr/api/ApiBag.java
index 2c14324..12b8db9 100644
--- a/solr/core/src/java/org/apache/solr/api/ApiBag.java
+++ b/solr/core/src/java/org/apache/solr/api/ApiBag.java
@@ -61,7 +61,7 @@ public class ApiBag {
   private final boolean isCoreSpecific;
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  private final Map<String, PathTrie<Api>> apis = new ConcurrentHashMap<>(128, 0.75f, 6);
+  private final Map<String, PathTrie<Api>> apis = new ConcurrentHashMap<>(128, 0.75f, 10);
 
   public ApiBag(boolean isCoreSpecific) {
     this.isCoreSpecific = isCoreSpecific;
diff --git a/solr/core/src/java/org/apache/solr/core/SolrCore.java b/solr/core/src/java/org/apache/solr/core/SolrCore.java
index 1070b4f..9962b96 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCore.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCore.java
@@ -1647,6 +1647,21 @@ public final class SolrCore implements SolrInfoBean, Closeable {
 
       closer.collect(updateHandler);
 
+      closer.addCollect();
+
+      AtomicBoolean coreStateClosed = new AtomicBoolean(false);
+
+      closer.collect("SolrCoreState", () -> {
+        boolean closed = false;
+        if (updateHandler != null && updateHandler instanceof IndexWriterCloser && solrCoreState != null) {
+          closed = solrCoreState.decrefSolrCoreState((IndexWriterCloser) updateHandler);
+        } else {
+          closed = solrCoreState.decrefSolrCoreState(null);
+        }
+        coreStateClosed.set(closed);
+        return solrCoreState;
+      });
+
       closer.collect(searcherExecutor);
 
       closer.addCollect();
@@ -1680,21 +1695,6 @@ public final class SolrCore implements SolrInfoBean, Closeable {
 
       closer.addCollect();
 
-      AtomicBoolean coreStateClosed = new AtomicBoolean(false);
-
-      closer.collect("SolrCoreState", () -> {
-        boolean closed = false;
-        if (updateHandler != null && updateHandler instanceof IndexWriterCloser && solrCoreState != null) {
-          closed = solrCoreState.decrefSolrCoreState((IndexWriterCloser) updateHandler);
-        } else {
-          closed = solrCoreState.decrefSolrCoreState(null);
-        }
-        coreStateClosed.set(closed);
-        return solrCoreState;
-      });
-
-      closer.addCollect();
-
       closer.collect("CleanupOldIndexDirs", () -> {
         if (coreStateClosed.get()) cleanupOldIndexDirectories(false);
       });
diff --git a/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java b/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java
index af16316..a588f1e 100644
--- a/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java
+++ b/solr/core/src/test/org/apache/solr/core/CachingDirectoryFactoryTest.java
@@ -75,7 +75,7 @@ public class CachingDirectoryFactoryTest extends SolrTestCaseJ4 {
       incRefThread.start();
     }
 
-    Thread.sleep(TEST_NIGHTLY ? 30000 : 500);
+    Thread.sleep(TEST_NIGHTLY ? 30000 : 50);
 
     Thread closeThread = new Thread() {
       public void run() {
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/PathTrie.java b/solr/solrj/src/java/org/apache/solr/common/util/PathTrie.java
index 2fd84c2..a7d885a 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/PathTrie.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/PathTrie.java
@@ -19,6 +19,7 @@ package org.apache.solr.common.util;
 
 import static java.util.Collections.emptyList;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -29,8 +30,8 @@ import java.util.concurrent.ConcurrentHashMap;
  * like /collections/{collection}/shards/{shard}/{replica}
  */
 public class PathTrie<T> {
-  private final Set<String> reserved = ConcurrentHashMap.newKeySet(64);
-  private volatile Node root = new Node(emptyList(), null);
+  private final Set<String> reserved = new HashSet<>();
+  private volatile Node root = new Node(reserved, emptyList(), null);
 
   public PathTrie() {
   }
@@ -82,15 +83,15 @@ public class PathTrie<T> {
 
 
   public T lookup(String path, Map<String, String> templateValues) {
-    return root.lookup(getPathSegments(path), 0, templateValues);
+    return (T) root.lookup(getPathSegments(path), 0, templateValues);
   }
 
   public T lookup(List<String> path, Map<String, String> templateValues) {
-    return root.lookup(path, 0, templateValues);
+    return (T) root.lookup(path, 0, templateValues);
   }
 
   public T lookup(String path, Map<String, String> templateValues, Set<String> paths) {
-    return root.lookup(getPathSegments(path), 0, templateValues, paths);
+    return (T) root.lookup(getPathSegments(path), 0, templateValues, paths);
   }
 
   public static String templateName(String templateStr) {
@@ -100,13 +101,15 @@ public class PathTrie<T> {
 
   }
 
-  class Node {
+  static class Node {
+    private final Set<String> reserved;
     String name;
     Map<String, Node> children;
-    T obj;
+    Object obj;
     String templateName;
 
-    Node(List<String> path, T o) {
+    Node(Set<String> reserved, List<String> path, Object o) {
+      this.reserved = reserved;
       if (path.isEmpty()) {
         obj = o;
         return;
@@ -118,7 +121,7 @@ public class PathTrie<T> {
     }
 
 
-    private void insert(List<String> path, T o) {
+    private void insert(List<String> path, Object o) {
       String part = path.get(0);
       Node matchedChild = null;
       if ("*".equals(name)) {
@@ -131,7 +134,7 @@ public class PathTrie<T> {
 
       matchedChild = children.get(key);
       if (matchedChild == null) {
-        children.put(key, matchedChild = new Node(path, o));
+        children.put(key, matchedChild = new Node(reserved, path, o));
       }
       if (varName != null) {
         if (!matchedChild.templateName.equals(varName)) {
@@ -165,7 +168,7 @@ public class PathTrie<T> {
     }
 
 
-    public T lookup(List<String> pieces, int i, Map<String, String> templateValues) {
+    public Object lookup(List<String> pieces, int i, Map<String, String> templateValues) {
       return lookup(pieces, i, templateValues, null);
 
     }
@@ -176,7 +179,7 @@ public class PathTrie<T> {
      * @param templateVariables The mapping of template variable to its value
      * @param availableSubPaths If not null , available sub paths will be returned in this set
      */
-    public T lookup(List<String> pathSegments, int index, Map<String, String> templateVariables, Set<String> availableSubPaths) {
+    public Object lookup(List<String> pathSegments, int index, Map<String, String> templateVariables, Set<String> availableSubPaths) {
       if (templateName != null) templateVariables.put(templateName, pathSegments.get(index - 1));
       if (pathSegments.size() < index + 1) {
         findAvailableChildren("", availableSubPaths);