You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by er...@apache.org on 2019/04/20 00:13:16 UTC

[lucene-solr] branch master updated: SOLR-13400: Replace Observable pattern in TransientSolrCoreCache

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

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


The following commit(s) were added to refs/heads/master by this push:
     new d87196c  SOLR-13400: Replace Observable pattern in TransientSolrCoreCache
d87196c is described below

commit d87196c1417800bb9fe59bfeb66597d08655a780
Author: Erick Erickson <Er...@gmail.com>
AuthorDate: Fri Apr 19 17:00:41 2019 -0700

    SOLR-13400: Replace Observable pattern in TransientSolrCoreCache
---
 solr/CHANGES.txt                                   |  5 +++
 .../java/org/apache/solr/core/CoreContainer.java   |  4 +++
 .../apache/solr/core/SolrCoreCloseListener.java    | 42 ----------------------
 .../src/java/org/apache/solr/core/SolrCores.java   | 18 ++--------
 .../apache/solr/core/TransientSolrCoreCache.java   | 29 ++-------------
 .../solr/core/TransientSolrCoreCacheDefault.java   | 16 ++-------
 6 files changed, 17 insertions(+), 97 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index d08112c..b0769b4 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -101,6 +101,9 @@ Upgrade Notes
 * SolrGangliaReporter has been removed from Solr because support for Ganglia has been removed from Dropwizard Metrics 4
   due to a transitive dependency on LGPL.
 
+* Custom TransientSolrCoreCache implementations no longer use the Observer/Observable pattern. To notify Solr that
+  a core has been aged out of the cache, call CoreContainer.queueCoreToClose(SolrCore). See SOLR-13400 for details.
+
 
 New Features
 ----------------------
@@ -294,6 +297,8 @@ Other Changes
 
 * SOLR-12461: Upgrade Dropwizard Metrics to 4.0.5 release. (ab)
 
+* SOLR-13400: Replace Observable pattern in TransientSolrCoreCache (Erick Erickson)
+
 ==================  8.0.0 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.
diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index fcd5daa..abaaf92 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -1827,6 +1827,10 @@ public class CoreContainer {
     return solrCores.isLoadedNotPendingClose(name);
   }
 
+  // Primarily for transient cores when a core is aged out.
+  public void queueCoreToClose(SolrCore coreToClose) {
+    solrCores.queueCoreToClose(coreToClose);
+  }
   /**
    * Gets a solr core descriptor for a core that is not loaded. Note that if the caller calls this on a
    * loaded core, the unloaded descriptor will be returned.
diff --git a/solr/core/src/java/org/apache/solr/core/SolrCoreCloseListener.java b/solr/core/src/java/org/apache/solr/core/SolrCoreCloseListener.java
deleted file mode 100644
index 4a5b41c..0000000
--- a/solr/core/src/java/org/apache/solr/core/SolrCoreCloseListener.java
+++ /dev/null
@@ -1,42 +0,0 @@
-/*
- * 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.core;
-
-import java.beans.PropertyChangeEvent;
-import java.beans.PropertyChangeListener;
-
-/**
- * Notifies observers implementing this class about cores that need to be closed.
- */
-public interface SolrCoreCloseListener extends PropertyChangeListener {
-
-  /**
-   * Called by TransientSolrCoreCache to notify the CoreContainer / SolrCores about cores that need to be closed.
-   * @param core Core that need to be queued for close
-   */
-  void queueCoreClose(SolrCore core);
-
-  @Override
-  default void propertyChange(PropertyChangeEvent evt) {
-    queueCoreClose((SolrCore) evt.getOldValue());
-  }
-
-}
-
-
-  
diff --git a/solr/core/src/java/org/apache/solr/core/SolrCores.java b/solr/core/src/java/org/apache/solr/core/SolrCores.java
index 43034a5..ad295f2 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrCores.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrCores.java
@@ -40,7 +40,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
 
 
-class SolrCores implements SolrCoreCloseListener {
+class SolrCores {
 
   private static Object modifyLock = new Object(); // for locking around manipulating any of the core maps.
   private final Map<String, SolrCore> cores = new LinkedHashMap<>(); // For "permanent" cores
@@ -532,21 +532,9 @@ class SolrCores implements SolrCoreCloseListener {
     return false;
   }
 
-  // Let transient cache implementation tell us when it ages out a core
-  @Override
-  public void queueCoreClose(SolrCore core) {
+  public void queueCoreToClose(SolrCore coreToClose) {
     synchronized (modifyLock) {
-      // Erick Erickson debugging TestLazyCores. With this un-commented, we get no testLazyCores failures.
-//      SolrQueryRequest req = new LocalSolrQueryRequest(core, new ModifiableSolrParams());
-//      CommitUpdateCommand cmd = new CommitUpdateCommand(req, false);
-//      cmd.openSearcher = false;
-//      cmd.waitSearcher = false;
-//      try {
-//        core.getUpdateHandler().commit(cmd);
-//      } catch (IOException e) {
-//        log.warn("Caught exception trying to close a transient core, ignoring as it should be benign");
-//      }
-      pendingCloses.add(core); // Essentially just queue this core up for closing.
+      pendingCloses.add(coreToClose); // Essentially just queue this core up for closing.
       modifyLock.notifyAll(); // Wakes up closer thread too
     }
   }
diff --git a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCache.java b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCache.java
index 3af7e23..278590e 100644
--- a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCache.java
+++ b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCache.java
@@ -18,7 +18,6 @@
 package org.apache.solr.core;
 
 
-import java.beans.PropertyChangeSupport;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
@@ -115,36 +114,14 @@ public abstract class TransientSolrCoreCache {
   /**
    * Must be called in order to free resources!
    */
-  public abstract void close();
+  public  void close(){
+    // Nothing to do currently
+  };
 
 
   // These two methods allow custom implementations to communicate arbitrary information as necessary.
   public abstract int getStatus(String coreName);
   public abstract void setStatus(String coreName, int status);
-  
-  
-  /**
-   * Registers a listener to be notified when a core should close
-   */
-  protected final void registerCoreCloseListener(SolrCoreCloseListener listener) {
-    pcs.addPropertyChangeListener(listener);
-  }
-  
-  /**
-   * Removes a listener registered by {@link #registerCoreCloseListener(SolrCoreCloseListener)}
-   */
-  protected final void removeCoreCloseListener(SolrCoreCloseListener listener) {
-    pcs.removePropertyChangeListener(listener);
-  }
-  
-  /**
-   * Notifies all listeners to close a core that were previously registered using {@link #registerCoreCloseListener(SolrCoreCloseListener)}
-   */
-  protected final void notifyCoreCloseListeners(SolrCore core) {
-    pcs.firePropertyChange("core", core, null);
-  }
-  
-  private final PropertyChangeSupport pcs = new  PropertyChangeSupport(this);
 }
 
 
diff --git a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheDefault.java b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheDefault.java
index 0b3db4b..84dd4e7 100644
--- a/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheDefault.java
+++ b/solr/core/src/java/org/apache/solr/core/TransientSolrCoreCacheDefault.java
@@ -35,7 +35,6 @@ public class TransientSolrCoreCacheDefault extends TransientSolrCoreCache {
 
   private int cacheSize = NodeConfig.NodeConfigBuilder.DEFAULT_TRANSIENT_CACHE_SIZE;
 
-  protected SolrCoreCloseListener coreCloseListener;
   protected CoreContainer coreContainer;
 
   protected final Map<String, CoreDescriptor> transientDescriptors = new LinkedHashMap<>();
@@ -48,8 +47,7 @@ public class TransientSolrCoreCacheDefault extends TransientSolrCoreCache {
    */
   public TransientSolrCoreCacheDefault(final CoreContainer container) {
     this.coreContainer = container;
-    this.coreCloseListener = coreContainer.solrCores;
-    
+
     NodeConfig cfg = container.getNodeConfig();
     if (cfg.getTransientCachePluginInfo() == null) {
       // Still handle just having transientCacheSize defined in the body of solr.xml  not in a transient handler clause.
@@ -79,7 +77,6 @@ public class TransientSolrCoreCacheDefault extends TransientSolrCoreCache {
     }
 
     log.info("Allocating transient cache for {} transient cores", cacheSize);
-    this.registerCoreCloseListener(this.coreCloseListener);
     // it's possible for cache
     if (cacheSize < 0) { // Trap old flag
       cacheSize = Integer.MAX_VALUE;
@@ -92,7 +89,7 @@ public class TransientSolrCoreCacheDefault extends TransientSolrCoreCache {
         if (size() > cacheSize) {
           SolrCore coreToClose = eldest.getValue();
           log.info("Closing transient core [{}]", coreToClose.getName());
-          notifyCoreCloseListeners(coreToClose);
+          coreContainer.queueCoreToClose(coreToClose);
           return true;
         }
         return false;
@@ -177,15 +174,6 @@ public class TransientSolrCoreCacheDefault extends TransientSolrCoreCache {
     return ret;
   }
 
-  /**
-   * Must be called in order to free resources!
-   */
-  @Override
-  public void close() {
-    this.removeCoreCloseListener(this.coreCloseListener);
-  }
-
-
   // For custom implementations to communicate arbitrary information as necessary.
   @Override
   public int getStatus(String coreName) { return 0; } //no_op for default handler.