diff --git a/CHANGES.txt b/CHANGES.txt index acf8936097d..ee19517b11f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -358,6 +358,9 @@ Changes in runtime behavior 1.3 release. Speciffically, solr.xml has replaced multicore.xml, and uses a slightly different syntax. The solrj classes: MultiCore{Request/Response/Params} have been renamed: CoreAdmin{Request/Response/Params} (hossman, ryan, Henri Biestro) + + 3. SOLR-647: reference count the SolrCore uses to prevent a premature + close while a core is still in use. (Henri Biestro, Noble Paul, yonik) Optimizations 1. SOLR-276: improve JSON writer speed. (yonik) diff --git a/client/java/solrj/src/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java b/client/java/solrj/src/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java index 1d3320475ff..4465b346e71 100644 --- a/client/java/solrj/src/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java +++ b/client/java/solrj/src/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java @@ -36,6 +36,7 @@ import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrCore; +import org.apache.solr.core.CoreDescriptor; import org.apache.solr.request.BinaryResponseWriter; import org.apache.solr.request.QueryResponseWriter; import org.apache.solr.request.SolrQueryRequest; @@ -54,38 +55,46 @@ import org.apache.solr.servlet.SolrRequestParsers; */ public class EmbeddedSolrServer extends SolrServer { - - protected final CoreContainer multicore; // either cores - protected final SolrCore core; // or single core - protected final String coreName; // use CoreContainer registry - + protected final CoreContainer coreContainer; + protected final String coreName; private final SolrRequestParsers _parser; + /** + * Use the other constructor using a CoreContainer and a name. + * @param core + * @deprecated + */ + @Deprecated public EmbeddedSolrServer( SolrCore core ) { if ( core == null ) { throw new NullPointerException("SolrCore instance required"); } - this.core = core; - if (core.getCoreDescriptor() != null) { - this.multicore = core.getCoreDescriptor().getMultiCore(); - this.coreName = core.getCoreDescriptor().getName(); - } else { - this.multicore = null; - this.coreName = null; - } + CoreDescriptor dcore = core.getCoreDescriptor(); + if (dcore == null) + throw new NullPointerException("CoreDescriptor required"); + + CoreContainer cores = dcore.getCoreContainer(); + if (cores == null) + throw new NullPointerException("CoreContainer required"); + + coreName = dcore.getName(); + coreContainer = cores; _parser = new SolrRequestParsers( null ); } - public EmbeddedSolrServer( CoreContainer multicore, String coreName ) + /** + * Creates a SolrServer. + * @param coreContainer the core container + * @param coreName the core name + */ + public EmbeddedSolrServer( CoreContainer coreContainer, String coreName ) { - if ( multicore == null ) { + if ( coreContainer == null ) { throw new NullPointerException("CoreContainer instance required"); } - this.core = null; - this.multicore = multicore; + this.coreContainer = coreContainer; this.coreName = coreName == null? "" : coreName; - _parser = new SolrRequestParsers( null ); } @@ -98,15 +107,10 @@ public class EmbeddedSolrServer extends SolrServer } // Check for cores action - SolrCore core = this.core; - if( core == null ) - core = multicore.getCore( coreName ); - // solr-647 - //else - // core = core.open(); + SolrCore core = coreContainer.getCore( coreName ); if( core == null ) { throw new SolrException( SolrException.ErrorCode.SERVER_ERROR, - coreName == null? "No core": "No such core: " + coreName ); + "No such core: " + coreName ); } SolrParams params = request.getParams(); @@ -126,14 +130,13 @@ public class EmbeddedSolrServer extends SolrServer } // Perhaps the path is to manage the cores if( handler == null && - multicore != null && - path.equals( multicore.getAdminPath() ) ) { - handler = multicore.getMultiCoreHandler(); + coreContainer != null && + path.equals( coreContainer.getAdminPath() ) ) { + handler = coreContainer.getMultiCoreHandler(); } } if( handler == null ) { - // solr-647 - // core.close(); + core.close(); throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "unknown handler: "+path ); } @@ -158,8 +161,7 @@ public class EmbeddedSolrServer extends SolrServer throw new SolrServerException( ex ); } finally { - // solr-647 - // core.close(); + core.close(); } } diff --git a/client/java/solrj/test/org/apache/solr/client/solrj/embedded/LargeVolumeEmbeddedTest.java b/client/java/solrj/test/org/apache/solr/client/solrj/embedded/LargeVolumeEmbeddedTest.java index 59d9a11fe7b..e5235624a19 100644 --- a/client/java/solrj/test/org/apache/solr/client/solrj/embedded/LargeVolumeEmbeddedTest.java +++ b/client/java/solrj/test/org/apache/solr/client/solrj/embedded/LargeVolumeEmbeddedTest.java @@ -45,6 +45,6 @@ public class LargeVolumeEmbeddedTest extends LargeVolumeTestBase { @Override protected SolrServer createNewSolrServer() { - return new EmbeddedSolrServer( h.getCore() ); + return new EmbeddedSolrServer( h.getCoreContainer(), "" ); } } diff --git a/client/java/solrj/test/org/apache/solr/client/solrj/embedded/SolrExampleEmbeddedTest.java b/client/java/solrj/test/org/apache/solr/client/solrj/embedded/SolrExampleEmbeddedTest.java index 7b15c44ceb1..36c57af1dd8 100644 --- a/client/java/solrj/test/org/apache/solr/client/solrj/embedded/SolrExampleEmbeddedTest.java +++ b/client/java/solrj/test/org/apache/solr/client/solrj/embedded/SolrExampleEmbeddedTest.java @@ -47,6 +47,6 @@ public class SolrExampleEmbeddedTest extends SolrExampleTests { @Override protected SolrServer createNewSolrServer() { - return new EmbeddedSolrServer( h.getCore() ); + return new EmbeddedSolrServer( h.getCoreContainer(), "" ); } } diff --git a/src/java/org/apache/solr/core/CoreContainer.java b/src/java/org/apache/solr/core/CoreContainer.java index 35736badda9..66a55908648 100644 --- a/src/java/org/apache/solr/core/CoreContainer.java +++ b/src/java/org/apache/solr/core/CoreContainer.java @@ -25,9 +25,7 @@ import java.io.IOException; import java.io.OutputStreamWriter; import java.io.Writer; import java.nio.channels.FileChannel; -import java.util.Collection; -import java.util.LinkedHashMap; -import java.util.Map; +import java.util.*; import java.util.logging.Logger; import javax.xml.parsers.ParserConfigurationException; @@ -36,6 +34,7 @@ import javax.xml.xpath.XPathConstants; import org.apache.solr.common.SolrException; import org.apache.solr.common.util.DOMUtil; import org.apache.solr.common.util.XML; +import org.apache.solr.common.util.StrUtils; import org.apache.solr.handler.admin.CoreAdminHandler; import org.apache.solr.schema.IndexSchema; import org.w3c.dom.Node; @@ -51,7 +50,7 @@ public class CoreContainer { protected static Logger log = Logger.getLogger(CoreContainer.class.getName()); - protected final Map cores = new LinkedHashMap(); + protected final Map cores = new LinkedHashMap(); protected boolean persistent = false; protected String adminPath = null; protected String managementPath = null; @@ -69,8 +68,7 @@ public class CoreContainer public static class Initializer { protected String solrConfigFilename = null; protected boolean abortOnConfigurationError = true; - protected String managementPath = null; - + public boolean isAbortOnConfigurationError() { return abortOnConfigurationError; } @@ -86,14 +84,6 @@ public class CoreContainer public void setSolrConfigFilename(String solrConfigFilename) { this.solrConfigFilename = solrConfigFilename; } - - public String getManagementPath() { - return managementPath; - } - - public void setManagementPath(String managementPath) { - this.managementPath = managementPath; - } // core container instantiation public CoreContainer initialize() throws IOException, ParserConfigurationException, SAXException { @@ -116,16 +106,13 @@ public class CoreContainer solrConfigFilename = cores.getConfigFile().getName(); } else { // perform compatibility init - cores = new CoreContainer(); - cores.loader = new SolrResourceLoader(instanceDir); + cores = new CoreContainer(new SolrResourceLoader(instanceDir)); SolrConfig cfg = solrConfigFilename == null ? new SolrConfig() : new SolrConfig(solrConfigFilename); - CoreDescriptor dcore = new CoreDescriptor(cores); - dcore.init("", cfg.getResourceLoader().getInstanceDir()); + CoreDescriptor dcore = new CoreDescriptor(cores, "", cfg.getResourceLoader().getInstanceDir()); SolrCore singlecore = new SolrCore(null, null, cfg, null, dcore); - dcore.setCore(singlecore); abortOnConfigurationError = cfg.getBool( "abortOnConfigurationError", abortOnConfigurationError); - cores.register(dcore); + cores.register("", singlecore, false); cores.setPersistent(false); solrConfigFilename = cfg.getName(); } @@ -147,6 +134,14 @@ public class CoreContainer this.load(dir, configFile); } + /** + * Minimal CoreContainer constructor. + * @param loader the CoreContainer resource loader + */ + public CoreContainer(SolrResourceLoader loader) { + this.loader = loader; + } + //------------------------------------------------------------------- // Initialization / Cleanup //------------------------------------------------------------------- @@ -165,54 +160,60 @@ public class CoreContainer FileInputStream cfgis = new FileInputStream(configFile); try { Config cfg = new Config(loader, null, cfgis, null); - + persistent = cfg.getBool( "solr/@persistent", false ); libDir = cfg.get( "solr/@sharedLib", null); adminPath = cfg.get( "solr/cores/@adminPath", null ); managementPath = cfg.get("solr/cores/@managementPath", null ); - + if (libDir != null) { // relative dir to conf File f = new File(dir, libDir); - libDir = f.getPath(); + libDir = f.getPath(); log.info( "loading shared library: "+f.getAbsolutePath() ); libLoader = SolrResourceLoader.createClassLoader(f, null); } - + if( adminPath != null ) { coreAdminHandler = this.createMultiCoreHandler(); } - + NodeList nodes = (NodeList)cfg.evaluate("solr/cores/core", XPathConstants.NODESET); - synchronized (cores) { - for (int i=0; i aliases = StrUtils.splitSmart(names,','); + String name = aliases.get(0); + CoreDescriptor p = new CoreDescriptor(this, name, DOMUtil.getAttr(node, "instanceDir", null)); + + // deal with optional settings + String opt = DOMUtil.getAttr(node, "config", null); + if (opt != null) { + p.setConfigName(opt); } - catch (Throwable ex) { - SolrConfig.severeErrors.add( ex ); - SolrException.logOnce(log,null,ex); + opt = DOMUtil.getAttr(node, "schema", null); + if (opt != null) { + p.setSchemaName(opt); } + + SolrCore core = create(p); + + for (int a=1; a= 0 || name.indexOf( '\\' ) >= 0 ){ throw new RuntimeException( "Invalid core name: "+name ); } - CoreDescriptor old = null; + SolrCore old = null; synchronized (cores) { - old = cores.put(name, descr); + old = cores.put(name, core); + core.setName(name); } - if( old == null ) { + + + if( old == null || old == core) { log.info( "registering core: "+name ); return null; - } + } else { log.info( "replacing core: "+name ); + if (!returnPrev) { + old.close(); + } return old; } } + /** - * Creates a new core based on a descriptor. + * Creates a new core based on a descriptor but does not register it. * * @param dcore a core descriptor * @return the newly created core @@ -293,10 +295,6 @@ public class CoreContainer SolrConfig config = new SolrConfig(solrLoader, dcore.getConfigName(), null); IndexSchema schema = new IndexSchema(config, dcore.getSchemaName(), null); SolrCore core = new SolrCore(dcore.getName(), null, config, schema, dcore); - dcore.setCore(core); - - // Register the new core - CoreDescriptor old = this.register(dcore); return core; } @@ -304,37 +302,39 @@ public class CoreContainer * @return a Collection of registered SolrCores */ public Collection getCores() { - java.util.List l = new java.util.ArrayList(); + List lst = new ArrayList(); synchronized (cores) { - for(CoreDescriptor descr : this.cores.values()) { - if (descr.getCore() != null) - l.add(descr.getCore()); - } + lst.addAll(this.cores.values()); } - return l; - } - - /** - * @return a Collection of registered CoreDescriptors - */ - public Collection getDescriptors() { - java.util.List l = new java.util.ArrayList(); - synchronized (cores) { - l.addAll(cores.values()); - } - return l; + return lst; } /** - * @return the CoreDescriptor registered under that name + * @return a Collection of the names that cores are mapped to */ - public CoreDescriptor getDescriptor(String name) { - synchronized(cores) { - return cores.get( name ); + public Collection getCoreNames() { + List lst = new ArrayList(); + synchronized (cores) { + lst.addAll(this.cores.keySet()); } + return lst; } - - + + /** + * @return a Collection of the names that a specific core is mapped to. + */ + public Collection getCoreNames(SolrCore core) { + List lst = new ArrayList(); + synchronized (cores) { + for (Map.Entry entry : cores.entrySet()) { + if (core == entry.getValue()) { + lst.add(entry.getKey()); + } + } + } + return lst; + } + // ---------------- Core name related methods --------------- /** * Recreates a SolrCore. @@ -348,12 +348,14 @@ public class CoreContainer */ public void reload(String name) throws ParserConfigurationException, IOException, SAXException { + SolrCore core; synchronized(cores) { - CoreDescriptor dcore = cores.get(name); - if (dcore == null) - throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "No such core: " + name ); - create(new CoreDescriptor(dcore)); + core = cores.get(name); } + if (core == null) + throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "No such core: " + name ); + + register(name, create(core.getCoreDescriptor()), false); } @@ -367,36 +369,27 @@ public class CoreContainer throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "Can not swap unnamed cores." ); } synchronized( cores ) { - CoreDescriptor c0 = cores.get(n0); + SolrCore c0 = cores.get(n0); + SolrCore c1 = cores.get(n1); if (c0 == null) throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "No such core: " + n0 ); - CoreDescriptor c1 = cores.get(n1); if (c1 == null) throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "No such core: " + n1 ); cores.put(n0, c1); cores.put(n1, c0); - c0.setName( n1 ); - if (c0.getCore() != null) - c0.getCore().setName(n1); - c1.setName( n0 ); - if (c1.getCore() != null) - c1.getCore().setName(n0); - log.info( "swaped: "+c0.getName() + " with " + c1.getName() ); + + c0.setName(n1); + c1.setName(n0); } + + + log.info("swaped: "+n0 + " with " + n1); } - /** Removes & closes a registered core. */ - public void remove( String name ) { + /** Removes and returns registered core w/o decrementing it's reference count */ + public SolrCore remove( String name ) { synchronized(cores) { - CoreDescriptor dcore = cores.remove( name ); - if (dcore == null) { - return; - } - - SolrCore core = dcore.getCore(); - if (core != null) { - core.close(); - } + return cores.remove( name ); } } @@ -408,15 +401,10 @@ public class CoreContainer */ public SolrCore getCore(String name) { synchronized(cores) { - CoreDescriptor dcore = cores.get(name); - SolrCore core = null; - if (dcore != null) - core = dcore.getCore(); - return core; -// solr-647 -// if (core != null) -// return core.open(); -// return null; + SolrCore core = cores.get(name); + if (core != null) + core.open(); // increment the ref count while still synchronized + return core; } } @@ -433,20 +421,17 @@ public class CoreContainer * Ensures there is a valid core to handle MultiCore admin taks and * increase its refcount. * @return the acquired admin core, null if no core is available - */ + */ public SolrCore getAdminCore() { synchronized (cores) { SolrCore core = adminCore != null ? adminCore.get() : null; -// solr-647 -// if (core != null) -// core = core.open(); - if (core == null) { - for (CoreDescriptor descr : this.cores.values()) { - core = descr.getCore(); -// solr-647 -// if (core != null) -// core = core.open(); - if (core != null) { + if (core != null) { + core.open(); + } else { + for (SolrCore c : cores.values()) { + if (c != null) { + core = c; + core.open(); break; } } @@ -517,7 +502,7 @@ public class CoreContainer public void persist() { persistFile(null); } - + /** Persists the cores config file in a user provided file. */ public void persistFile(File file) { File tmpFile = null; @@ -527,7 +512,6 @@ public class CoreContainer file = tmpFile = File.createTempFile("solr", ".xml", configFile.getParentFile()); } java.io.FileOutputStream out = new java.io.FileOutputStream(file); - synchronized(cores) { Writer writer = new BufferedWriter(new OutputStreamWriter(out, "UTF-8")); persist(writer); writer.flush(); @@ -540,7 +524,6 @@ public class CoreContainer else fileCopy(tmpFile, configFile); } - } } catch(java.io.FileNotFoundException xnf) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, xnf); @@ -578,20 +561,35 @@ public class CoreContainer writer.write('\''); writer.write(">\n"); + Map> aliases = new HashMap>(); + synchronized(cores) { - for (Map.Entry entry : cores.entrySet()) { - persist(writer, entry.getValue()); + for (Map.Entry entry : cores.entrySet()) { + String name = entry.getKey(); + LinkedList a = aliases.get(entry.getValue()); + if (a==null) a = new LinkedList(); + if (name.equals(entry.getValue().getName())) { + a.addFirst(name); + } else { + a.addLast(name); + } + aliases.put(entry.getValue(), a); } } + + for (Map.Entry> entry : aliases.entrySet()) { + persist(writer, entry.getValue(), entry.getKey().getCoreDescriptor()); + } + writer.write("\n"); writer.write("\n"); } /** Writes the cores configuration node for a given core. */ - void persist(Writer writer, CoreDescriptor dcore) throws IOException { + void persist(Writer writer, List aliases, CoreDescriptor dcore) throws IOException { writer.write(" + * This should always be called when the core is obtained through: + * @see CoreContainer.getCore + * @see CoreContainer.getAdminCore + *

+ * The actual close is performed if the core usage count is 1. + * (A core is created with a usage count of 1). + * If usage count is > 1, the usage count is decreased by 1. + * If usage count is < 0, this is an error and a runtime exception is thrown. */ public void close() { - log.info(logid+" CLOSING SolrCore!"); + int count = refCount.decrementAndGet(); + if (count > 0) return; + if (count < 0) { + //throw new RuntimeException("Too many closes on " + this); + log.severe("Too many close {count:"+count+"} on " + this + ". Please report this exception to solr-user@lucene.apache.org"); + return; + } + log.info(logid+" CLOSING SolrCore " + this); try { infoRegistry.clear(); } catch (Exception e) { @@ -602,16 +628,22 @@ public final class SolrCore { if( closeHooks != null ) { for( CloseHook hook : closeHooks ) { hook.close( this ); - } + } } } - public boolean isClosed() { - return _searcher == null; + /** Current core usage count. */ + public int getOpenCount() { + return refCount.get(); } - @Override - protected void finalize() { close(); } + /** Whether this core is closed. */ + public boolean isClosed() { + return refCount.get() <= 0; + } + + // this can cause an extra close + // protected void finalize() { close(); } private List closeHooks = null; @@ -1380,6 +1412,45 @@ public final class SolrCore { public CoreDescriptor getCoreDescriptor() { return coreDescriptor; } + + + ///////////////////////////////////////////////////////////////////// + // SolrInfoMBean stuff: Statistics and Module Info + ///////////////////////////////////////////////////////////////////// + + public String getVersion() { + return SolrCore.version; + } + + public String getDescription() { + return "SolrCore"; + } + + public Category getCategory() { + return Category.CORE; + } + + public String getSourceId() { + return "$Id:$"; + } + + public String getSource() { + return "$URL:$"; + } + + public URL[] getDocs() { + return null; + } + + public NamedList getStatistics() { + NamedList lst = new SimpleOrderedMap(); + lst.add("coreName", name==null ? "(null)" : name); + lst.add("startTime", new Date(startTime)); + lst.add("refCount", getOpenCount()); + lst.add("aliases", getCoreDescriptor().getCoreContainer().getCoreNames(this)); + return lst; + } + } diff --git a/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java b/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java index f88c91d1c6c..5fe9b3be595 100644 --- a/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java +++ b/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java @@ -90,9 +90,8 @@ public abstract class CoreAdminHandler extends RequestHandlerBase switch(action) { case CREATE: { - CoreDescriptor dcore = new CoreDescriptor(cores); - dcore.init(params.get(CoreAdminParams.NAME), - params.get(CoreAdminParams.INSTANCE_DIR)); + String name = params.get(CoreAdminParams.NAME); + CoreDescriptor dcore = new CoreDescriptor(cores, name, params.get(CoreAdminParams.INSTANCE_DIR)); // fillup optional parameters String opts = params.get(CoreAdminParams.CONFIG); @@ -104,6 +103,7 @@ public abstract class CoreAdminHandler extends RequestHandlerBase dcore.setSchemaName(opts); SolrCore core = cores.create(dcore); + cores.register(name, core,false); rsp.add("core", core.getName()); do_persist = cores.isPersistent(); break; @@ -112,9 +112,8 @@ public abstract class CoreAdminHandler extends RequestHandlerBase case STATUS: { NamedList status = new SimpleOrderedMap(); if( cname == null ) { - for (CoreDescriptor d : cores.getDescriptors()) { - cname = d.getName(); - status.add(d.getName(), getCoreStatus( cores, cname ) ); + for (SolrCore core : cores.getCores()) { + status.add(core.getName(), getCoreStatus( cores, cname ) ); } } else { @@ -172,8 +171,7 @@ public abstract class CoreAdminHandler extends RequestHandlerBase info.add("index", LukeRequestHandler.getIndexInfo(searcher.get().getReader(), false)); searcher.decref(); } finally { - // solr-647 - // core.close(); + core.close(); } } return info; diff --git a/src/java/org/apache/solr/util/TestHarness.java b/src/java/org/apache/solr/util/TestHarness.java index 8ee6ee0eef8..35ed6715d4f 100644 --- a/src/java/org/apache/solr/util/TestHarness.java +++ b/src/java/org/apache/solr/util/TestHarness.java @@ -21,6 +21,9 @@ import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.XML; import org.apache.solr.core.SolrConfig; import org.apache.solr.core.SolrCore; +import org.apache.solr.core.CoreContainer; +import org.apache.solr.core.CoreDescriptor; +import org.apache.solr.core.SolrResourceLoader; import org.apache.solr.handler.XmlUpdateRequestHandler; import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.QueryResponseWriter; @@ -32,6 +35,7 @@ import org.xml.sax.SAXException; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; import javax.xml.xpath.XPath; import javax.xml.xpath.XPathConstants; import javax.xml.xpath.XPathExpressionException; @@ -60,7 +64,7 @@ import java.util.Map; * @version $Id:$ */ public class TestHarness { - + protected CoreContainer container; private SolrCore core; private XPath xpath = XPathFactory.newInstance().newXPath(); private DocumentBuilder builder; @@ -123,8 +127,19 @@ public class TestHarness { public TestHarness( String dataDirectory, SolrConfig solrConfig, IndexSchema indexSchema) { + this("", new Initializer("", dataDirectory, solrConfig, indexSchema)); + } + + public TestHarness(String coreName, CoreContainer.Initializer init) { try { - core = new SolrCore( null, dataDirectory, solrConfig, indexSchema, null); + container = init.initialize(); + if (coreName == null) + coreName = ""; + // get the core & decrease its refcount: + // the container holds the core for the harness lifetime + core = container.getCore(coreName); + if (core != null) + core.close(); builder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); updater = new XmlUpdateRequestHandler(); @@ -133,6 +148,42 @@ public class TestHarness { throw new RuntimeException(e); } } + + // Creates a container based on infos needed to create one core + static class Initializer extends CoreContainer.Initializer { + String coreName; + String dataDirectory; + SolrConfig solrConfig; + IndexSchema indexSchema; + public Initializer(String coreName, + String dataDirectory, + SolrConfig solrConfig, + IndexSchema indexSchema) { + if (coreName == null) + coreName = ""; + this.coreName = coreName; + this.dataDirectory = dataDirectory; + this.solrConfig = solrConfig; + this.indexSchema = indexSchema; + } + public String getCoreName() { + return coreName; + } + @Override + public CoreContainer initialize() { + CoreContainer container = new CoreContainer(new SolrResourceLoader(SolrResourceLoader.locateInstanceDir())); + CoreDescriptor dcore = new CoreDescriptor(container, coreName, solrConfig.getResourceLoader().getInstanceDir()); + dcore.setConfigName(solrConfig.getResourceName()); + dcore.setSchemaName(indexSchema.getResourceName()); + SolrCore core = new SolrCore( null, dataDirectory, solrConfig, indexSchema, dcore); + container.register(coreName, core, false); + return container; + } + } + + public CoreContainer getCoreContainer() { + return container; + } public SolrCore getCore() { return core; @@ -152,9 +203,7 @@ public class TestHarness { StringReader req = new StringReader(xml); StringWriter writer = new StringWriter(32000); - - // This relies on the fact that SolrCore.getSolrCore() uses the - // last instantiated SolrCore. + updater.doLegacyUpdate(req, writer); return writer.toString(); } @@ -321,7 +370,17 @@ public class TestHarness { * Shuts down and frees any resources */ public void close() { - core.close(); + if (container != null) { + for (SolrCore c : container.getCores()) { + if (c.getOpenCount() > 1) + throw new RuntimeException("SolrCore.getOpenCount()=="+core.getOpenCount()); + } + } + + if (container != null) { + container.shutdown(); + container = null; + } } /** diff --git a/src/test/org/apache/solr/core/SolrCoreTest.java b/src/test/org/apache/solr/core/SolrCoreTest.java index 88a85ce0016..afb2b579d6a 100755 --- a/src/test/org/apache/solr/core/SolrCoreTest.java +++ b/src/test/org/apache/solr/core/SolrCoreTest.java @@ -24,13 +24,14 @@ import org.apache.solr.request.SolrRequestHandler; import org.apache.solr.util.AbstractSolrTestCase; import org.apache.solr.util.plugin.SolrCoreAware; +import java.util.concurrent.*; +import java.util.*; public class SolrCoreTest extends AbstractSolrTestCase { public String getSchemaFile() { return "schema.xml"; } public String getSolrConfigFile() { return "solrconfig.xml"; } - + public void testRequestHandlerRegistry() { - // property values defined in build.xml SolrCore core = h.getCore(); EmptyRequestHandler handler1 = new EmptyRequestHandler(); @@ -46,7 +47,8 @@ public class SolrCoreTest extends AbstractSolrTestCase { } public void testClose() throws Exception { - SolrCore core = h.getCore(); + final CoreContainer cores = h.getCoreContainer(); + SolrCore core = cores.getCore(""); ClosingRequestHandler handler1 = new ClosingRequestHandler(); handler1.inform( core ); @@ -56,10 +58,116 @@ public class SolrCoreTest extends AbstractSolrTestCase { assertNull( old ); // should not be anything... assertEquals( core.getRequestHandlers().get( path ), handler1 ); core.close(); + cores.shutdown(); assertTrue("Handler not closed", handler1.closed == true); } + + public void testRefCount() throws Exception { + SolrCore core = h.getCore(); + assertTrue("Refcount != 1", core.getOpenCount() == 1); + + final CoreContainer cores = h.getCoreContainer(); + SolrCore c1 = cores.getCore(""); + assertTrue("Refcount != 2", core.getOpenCount() == 2); + + ClosingRequestHandler handler1 = new ClosingRequestHandler(); + handler1.inform( core ); + + String path = "/this/is A path /that won't be registered!"; + SolrRequestHandler old = core.registerRequestHandler( path, handler1 ); + assertNull( old ); // should not be anything... + assertEquals( core.getRequestHandlers().get( path ), handler1 ); + + SolrCore c2 = cores.getCore(""); + c1.close(); + assertTrue("Refcount < 1", core.getOpenCount() >= 1); + assertTrue("Handler is closed", handler1.closed == false); + + c1 = cores.getCore(""); + assertTrue("Refcount < 2", core.getOpenCount() >= 2); + assertTrue("Handler is closed", handler1.closed == false); + + c2.close(); + assertTrue("Refcount < 1", core.getOpenCount() >= 1); + assertTrue("Handler is closed", handler1.closed == false); + + c1.close(); + cores.shutdown(); + assertTrue("Refcount != 0", core.getOpenCount() == 0); + assertTrue("Handler not closed", core.isClosed() && handler1.closed == true); + } + + + public void testRefCountMT() throws Exception { + SolrCore core = h.getCore(); + assertTrue("Refcount != 1", core.getOpenCount() == 1); + + final ClosingRequestHandler handler1 = new ClosingRequestHandler(); + handler1.inform(core); + String path = "/this/is A path /that won't be registered!"; + SolrRequestHandler old = core.registerRequestHandler(path, handler1); + assertNull(old); // should not be anything... + assertEquals(core.getRequestHandlers().get(path), handler1); + + final int LOOP = 100; + final int MT = 16; + ExecutorService service = Executors.newFixedThreadPool(MT); + List> callees = new ArrayList>(MT); + final CoreContainer cores = h.getCoreContainer(); + for (int i = 0; i < MT; ++i) { + Callable call = new Callable() { + void yield(int n) { + try { + Thread.sleep(0, (n % 13 + 1) * 10); + } catch (InterruptedException xint) { + } + } + + public Integer call() { + SolrCore core = null; + int r = 0; + try { + for (int l = 0; l < LOOP; ++l) { + r += 1; + core = cores.getCore(""); + // sprinkle concurrency hinting... + yield(l); + assertTrue("Refcount < 1", core.getOpenCount() >= 1); + yield(l); + assertTrue("Refcount > 17", core.getOpenCount() <= 17); + yield(l); + assertTrue("Handler is closed", handler1.closed == false); + yield(l); + core.close(); + core = null; + yield(l); + } + return r; + } finally { + if (core != null) + core.close(); + } + } + }; + callees.add(call); + } + + List> results = service.invokeAll(callees); + for (Future result : results) { + assertTrue("loop=" + result.get() +" < " + LOOP, result.get() >= LOOP); + } + + cores.shutdown(); + assertTrue("Refcount != 0", core.getOpenCount() == 0); + assertTrue("Handler not closed", core.isClosed() && handler1.closed == true); + + service.shutdown(); + assertTrue("Running for too long...", service.awaitTermination(60, TimeUnit.SECONDS)); + } } + + class ClosingRequestHandler extends EmptyRequestHandler implements SolrCoreAware { boolean closed = false; diff --git a/src/webapp/src/org/apache/solr/servlet/SolrDispatchFilter.java b/src/webapp/src/org/apache/solr/servlet/SolrDispatchFilter.java index d619918038a..6dac95a043f 100644 --- a/src/webapp/src/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/src/webapp/src/org/apache/solr/servlet/SolrDispatchFilter.java @@ -285,6 +285,9 @@ public class SolrDispatchFilter implements Filter if( solrReq != null ) { solrReq.close(); } + if (core != null) { + core.close(); + } } } diff --git a/src/webapp/web/admin/index.jsp b/src/webapp/web/admin/index.jsp index 0f77872a665..f7fd5ad7001 100644 --- a/src/webapp/web/admin/index.jsp +++ b/src/webapp/web/admin/index.jsp @@ -21,6 +21,8 @@ <%-- $Name: $ --%> <%@ page import="java.util.Date" %> +<%@ page import="java.util.List" %> +<%@ page import="java.util.Collection" %> <%-- jsp:include page="header.jsp"/ --%> <%-- do a verbatim include so we can use the local vars --%> @@ -52,15 +54,12 @@ <%-- List the cores (that arent this one) so we can switch --%> -<% org.apache.solr.core.CoreContainer multicore = (org.apache.solr.core.CoreContainer)request.getAttribute("org.apache.solr.CoreContainer"); - if (multicore!=null) { - java.util.Collection cores = multicore.getCores(); -if (cores.size() > 1) {%>Cores:
<% - java.util.Iterator icore = cores.iterator(); - while (icore.hasNext()) { - SolrCore acore = icore.next(); - if (acore == core) continue; - %>[<%=acore.getName()%>]<% +<% org.apache.solr.core.CoreContainer cores = (org.apache.solr.core.CoreContainer)request.getAttribute("org.apache.solr.CoreContainer"); + if (cores!=null) { + Collection names = cores.getCoreNames(); + if (names.size() > 1) {%>Cores:
<% + for (String name : names) { + %>[<%=name%>]<% }%><% }}%>