From d87196c1417800bb9fe59bfeb66597d08655a780 Mon Sep 17 00:00:00 2001 From: Erick Erickson Date: Fri, 19 Apr 2019 17:00:41 -0700 Subject: [PATCH] SOLR-13400: Replace Observable pattern in TransientSolrCoreCache --- solr/CHANGES.txt | 5 +++ .../org/apache/solr/core/CoreContainer.java | 4 ++ .../solr/core/SolrCoreCloseListener.java | 42 ------------------- .../java/org/apache/solr/core/SolrCores.java | 18 ++------ .../solr/core/TransientSolrCoreCache.java | 29 ++----------- .../core/TransientSolrCoreCacheDefault.java | 16 +------ 6 files changed, 17 insertions(+), 97 deletions(-) delete mode 100644 solr/core/src/java/org/apache/solr/core/SolrCoreCloseListener.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index d08112c9d0e..b0769b45693 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 fcd5daa98ed..abaaf92e845 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 4a5b41cc4d3..00000000000 --- 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 43034a54fbe..ad295f23af5 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 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 3af7e23c3d6..278590e9ff6 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 0b3db4b631f..84dd4e7aaad 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 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.