bruno-roustant commented on a change in pull request #2066:
URL: https://github.com/apache/lucene-solr/pull/2066#discussion_r519224315



##########
File path: solr/core/src/java/org/apache/solr/core/SolrCores.java
##########
@@ -198,35 +188,91 @@ protected SolrCore putCore(CoreDescriptor cd, SolrCore 
core) {
    * 
    * Put another way, this will not return any names of cores that are lazily 
loaded but have not been called for yet
    * or are transient and either not loaded or have been swapped out.
-   * 
-   * @return List of currently loaded cores.
+   *
+   * @return A unsorted collection.
    */
-  Set<String> getLoadedCoreNames() {
-    Set<String> set;
-
+  Collection<String> getLoadedCoreNames() {
     synchronized (modifyLock) {
-      set = new TreeSet<>(cores.keySet());
-      if (getTransientCacheHandler() != null) {
-        set.addAll(getTransientCacheHandler().getLoadedCoreNames());
-      }
+      TransientSolrCoreCache transientCoreCache = getTransientCacheHandler();
+      Set<String> transientCoreNames = transientCoreCache == null ? 
Collections.emptySet() : transientCoreCache.getLoadedCoreNames();
+      return distinctSetsUnion(cores.keySet(), transientCoreNames);
     }
-    return set;
   }
+
   /**
-   * Gets a list of all cores, loaded and unloaded 
+   * Gets a collection of all cores names, loaded and unloaded.
+   * For efficiency, prefer to check {@link #getCoreDescriptor(String)} != 
null instead of {@link #getAllCoreNames()}.contains(String)
    *
-   * @return all cores names, whether loaded or unloaded, transient or 
permanent.
+   * @return A unsorted collection.
    */
   public Collection<String> getAllCoreNames() {
-    Set<String> set;
     synchronized (modifyLock) {
-      set = new TreeSet<>(cores.keySet());
-      if (getTransientCacheHandler() != null) {
-        set.addAll(getTransientCacheHandler().getAllCoreNames());
-      }
-      set.addAll(residentDesciptors.keySet());
+      TransientSolrCoreCache transientCoreCache = getTransientCacheHandler();
+      Set<String> transientCoreNames = transientCoreCache == null ? 
Collections.emptySet() : transientCoreCache.getAllCoreNames();
+      return distinctSetsUnion(residentDescriptors.keySet(), 
transientCoreNames);
+    }
+  }
+
+  /**
+   * Makes the union of two distinct sets.
+   */
+  private static <T> Collection<T> distinctSetsUnion(Set<T> set1, Set<T> set2) 
{
+    assert areSetsDistinct(set1, set2);

Review comment:
       Yes it must be a snapshot (like in getCores()). I'll add the comment.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to