do we really need the get to be synchronized (as I posted on the issue) ? On Mon, Aug 18, 2008 at 8:38 PM, <[EMAIL PROTECTED]> wrote: > Author: yonik > Date: Mon Aug 18 08:08:28 2008 > New Revision: 686780 > > URL: http://svn.apache.org/viewvc?rev=686780&view=rev > Log: > SOLR-647: refcount cores > > Modified: > lucene/solr/trunk/CHANGES.txt > > lucene/solr/trunk/client/java/solrj/src/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java > > lucene/solr/trunk/client/java/solrj/test/org/apache/solr/client/solrj/embedded/LargeVolumeEmbeddedTest.java > > lucene/solr/trunk/client/java/solrj/test/org/apache/solr/client/solrj/embedded/SolrExampleEmbeddedTest.java > lucene/solr/trunk/src/java/org/apache/solr/core/CoreContainer.java > lucene/solr/trunk/src/java/org/apache/solr/core/CoreDescriptor.java > lucene/solr/trunk/src/java/org/apache/solr/core/SolrCore.java > > lucene/solr/trunk/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java > lucene/solr/trunk/src/java/org/apache/solr/util/TestHarness.java > lucene/solr/trunk/src/test/org/apache/solr/core/SolrCoreTest.java > > lucene/solr/trunk/src/webapp/src/org/apache/solr/servlet/SolrDispatchFilter.java > lucene/solr/trunk/src/webapp/web/admin/index.jsp > > Modified: lucene/solr/trunk/CHANGES.txt > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/CHANGES.txt?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- lucene/solr/trunk/CHANGES.txt (original) > +++ lucene/solr/trunk/CHANGES.txt Mon Aug 18 08:08:28 2008 > @@ -358,6 +358,9 @@ > 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) > > Modified: > lucene/solr/trunk/client/java/solrj/src/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/client/java/solrj/src/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- > lucene/solr/trunk/client/java/solrj/src/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java > (original) > +++ > lucene/solr/trunk/client/java/solrj/src/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java > Mon Aug 18 08:08:28 2008 > @@ -36,6 +36,7 @@ > 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 @@ > */ > 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 @@ > } > > // 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 @@ > } > // 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 @@ > throw new SolrServerException( ex ); > } > finally { > - // solr-647 > - // core.close(); > + core.close(); > } > } > > > Modified: > lucene/solr/trunk/client/java/solrj/test/org/apache/solr/client/solrj/embedded/LargeVolumeEmbeddedTest.java > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/client/java/solrj/test/org/apache/solr/client/solrj/embedded/LargeVolumeEmbeddedTest.java?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- > lucene/solr/trunk/client/java/solrj/test/org/apache/solr/client/solrj/embedded/LargeVolumeEmbeddedTest.java > (original) > +++ > lucene/solr/trunk/client/java/solrj/test/org/apache/solr/client/solrj/embedded/LargeVolumeEmbeddedTest.java > Mon Aug 18 08:08:28 2008 > @@ -45,6 +45,6 @@ > @Override > protected SolrServer createNewSolrServer() > { > - return new EmbeddedSolrServer( h.getCore() ); > + return new EmbeddedSolrServer( h.getCoreContainer(), "" ); > } > } > > Modified: > lucene/solr/trunk/client/java/solrj/test/org/apache/solr/client/solrj/embedded/SolrExampleEmbeddedTest.java > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/client/java/solrj/test/org/apache/solr/client/solrj/embedded/SolrExampleEmbeddedTest.java?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- > lucene/solr/trunk/client/java/solrj/test/org/apache/solr/client/solrj/embedded/SolrExampleEmbeddedTest.java > (original) > +++ > lucene/solr/trunk/client/java/solrj/test/org/apache/solr/client/solrj/embedded/SolrExampleEmbeddedTest.java > Mon Aug 18 08:08:28 2008 > @@ -47,6 +47,6 @@ > @Override > protected SolrServer createNewSolrServer() > { > - return new EmbeddedSolrServer( h.getCore() ); > + return new EmbeddedSolrServer( h.getCoreContainer(), "" ); > } > } > > Modified: lucene/solr/trunk/src/java/org/apache/solr/core/CoreContainer.java > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/apache/solr/core/CoreContainer.java?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- lucene/solr/trunk/src/java/org/apache/solr/core/CoreContainer.java > (original) > +++ lucene/solr/trunk/src/java/org/apache/solr/core/CoreContainer.java Mon > Aug 18 08:08:28 2008 > @@ -25,9 +25,7 @@ > 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 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 @@ > { > protected static Logger log = > Logger.getLogger(CoreContainer.class.getName()); > > - protected final Map<String, CoreDescriptor> cores = new > LinkedHashMap<String, CoreDescriptor>(); > + protected final Map<String, SolrCore> cores = new LinkedHashMap<String, > SolrCore>(); > protected boolean persistent = false; > protected String adminPath = null; > protected String managementPath = null; > @@ -69,8 +68,7 @@ > 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 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 @@ > 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 @@ > 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 @@ > 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<nodes.getLength(); i++) { > - Node node = nodes.item(i); > - try { > - CoreDescriptor p = new CoreDescriptor(this); > - p.init(DOMUtil.getAttr(node, "name", null), > DOMUtil.getAttr(node, "instanceDir", null)); > - // deal with optional settings > - String opt = DOMUtil.getAttr(node, "config", null); > - if (opt != null) { > - p.setConfigName(opt); > - } > - opt = DOMUtil.getAttr(node, "schema", null); > - if (opt != null) { > - p.setSchemaName(opt); > - } > - CoreDescriptor old = cores.get(p.getName()); > - if (old != null && old.getName() != null && > old.getName().equals(p.getName())) { > - throw new RuntimeException( cfg.getName() + > - " registers multiple cores to the same name: " + p.name); > - } > - p.setCore(create(p)); > + > + for (int i=0; i<nodes.getLength(); i++) { > + Node node = nodes.item(i); > + try { > + String names = DOMUtil.getAttr(node, "name", null); > + List<String> 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<aliases.size(); a++) { > + core.open(); > + register(aliases.get(i), core, false); > + } > + > + register(name, core, false); > + } > + catch (Throwable ex) { > + SolrConfig.severeErrors.add( ex ); > + SolrException.logOnce(log,null,ex); > } > } > } > + > finally { > if (cfgis != null) { > try { cfgis.close(); } catch (Exception xany) {} > @@ -225,11 +226,8 @@ > */ > public void shutdown() { > synchronized(cores) { > - for(CoreDescriptor descriptor : cores.values()) { > - SolrCore core = descriptor.getCore(); > - if( core != null ) { > - core.close(); > - } > + for(SolrCore core : cores.values()) { > + core.close(); > } > cores.clear(); > } > @@ -239,40 +237,44 @@ > protected void finalize() { > shutdown(); > } > - > - // ---------------- CoreDescriptor related methods --------------- > + > /** > * Registers a SolrCore descriptor in the registry. > - * @param descr the Solr core descriptor > - * @return a previous descriptor having the same name if it existed, null > otherwise > + * @return a previous core having the same name if it existed and > returnPrev==true > */ > - public CoreDescriptor register( CoreDescriptor descr ) { > - if( descr == null ) { > + public SolrCore register(String name, SolrCore core, boolean returnPrev) { > + if( core == null ) { > throw new RuntimeException( "Can not register a null core." ); > } > - String name = descr.getName(); > - if( name == null || > + if( name == null || > name.indexOf( '/' ) >= 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 @@ > 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 @@ > * @return a Collection of registered SolrCores > */ > public Collection<SolrCore> getCores() { > - java.util.List<SolrCore> l = new java.util.ArrayList<SolrCore>(); > + List<SolrCore> lst = new ArrayList<SolrCore>(); > synchronized (cores) { > - for(CoreDescriptor descr : this.cores.values()) { > - if (descr.getCore() != null) > - l.add(descr.getCore()); > - } > + lst.addAll(this.cores.values()); > } > - return l; > + return lst; > } > - > + > /** > - * @return a Collection of registered CoreDescriptors > + * @return a Collection of the names that cores are mapped to > */ > - public Collection<CoreDescriptor> getDescriptors() { > - java.util.List<CoreDescriptor> l = new > java.util.ArrayList<CoreDescriptor>(); > - synchronized (cores) { > - l.addAll(cores.values()); > - } > - return l; > + public Collection<String> getCoreNames() { > + List<String> lst = new ArrayList<String>(); > + synchronized (cores) { > + lst.addAll(this.cores.keySet()); > + } > + return lst; > } > > /** > - * @return the CoreDescriptor registered under that name > + * @return a Collection of the names that a specific core is mapped to. > */ > - public CoreDescriptor getDescriptor(String name) { > - synchronized(cores) { > - return cores.get( name ); > + public Collection<String> getCoreNames(SolrCore core) { > + List<String> lst = new ArrayList<String>(); > + synchronized (cores) { > + for (Map.Entry<String,SolrCore> 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 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 @@ > 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 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 @@ > * 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 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 @@ > 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 @@ > else > fileCopy(tmpFile, configFile); > } > - } > } > catch(java.io.FileNotFoundException xnf) { > throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, xnf); > @@ -578,20 +561,35 @@ > writer.write('\''); > writer.write(">\n"); > > + Map<SolrCore, LinkedList<String>> aliases = new > HashMap<SolrCore,LinkedList<String>>(); > + > synchronized(cores) { > - for (Map.Entry<String, CoreDescriptor> entry : cores.entrySet()) { > - persist(writer, entry.getValue()); > + for (Map.Entry<String, SolrCore> entry : cores.entrySet()) { > + String name = entry.getKey(); > + LinkedList<String> a = aliases.get(entry.getValue()); > + if (a==null) a = new LinkedList<String>(); > + if (name.equals(entry.getValue().getName())) { > + a.addFirst(name); > + } else { > + a.addLast(name); > + } > + aliases.put(entry.getValue(), a); > } > } > + > + for (Map.Entry<SolrCore, LinkedList<String>> entry : aliases.entrySet()) > { > + persist(writer, entry.getValue(), entry.getKey().getCoreDescriptor()); > + } > + > writer.write("</cores>\n"); > writer.write("</solr>\n"); > } > > /** Writes the cores configuration node for a given core. */ > - void persist(Writer writer, CoreDescriptor dcore) throws IOException { > + void persist(Writer writer, List<String> aliases, CoreDescriptor dcore) > throws IOException { > writer.write(" <core"); > writer.write (" name='"); > - XML.escapeAttributeValue(dcore.getName(), writer); > + XML.escapeAttributeValue(StrUtils.join(aliases,','), writer); > writer.write("' instanceDir='"); > XML.escapeAttributeValue(dcore.getInstanceDir(), writer); > writer.write('\''); > > Modified: lucene/solr/trunk/src/java/org/apache/solr/core/CoreDescriptor.java > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/apache/solr/core/CoreDescriptor.java?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- lucene/solr/trunk/src/java/org/apache/solr/core/CoreDescriptor.java > (original) > +++ lucene/solr/trunk/src/java/org/apache/solr/core/CoreDescriptor.java Mon > Aug 18 08:08:28 2008 > @@ -28,22 +28,17 @@ > protected String instanceDir; > protected String configName; > protected String schemaName; > - protected SolrCore core = null; > private final CoreContainer coreContainer; > > - public CoreDescriptor(CoreContainer coreContainer) { > + public CoreDescriptor(CoreContainer coreContainer, String name, String > instanceDir) { > this.coreContainer = coreContainer; > - } > - > - /** Initialize defaults from instance directory. */ > - public void init(String name, String instanceDir) { > + this.name = name; > if (name == null) { > throw new RuntimeException("Core needs a name"); > } > if (instanceDir == null) { > throw new NullPointerException("Missing required \'instanceDir\'"); > } > - this.name = name; > if (!instanceDir.endsWith("/")) instanceDir = instanceDir + "/"; > this.instanceDir = instanceDir; > this.configName = getDefaultConfigName(); > @@ -51,10 +46,10 @@ > } > > public CoreDescriptor(CoreDescriptor descr) { > - this.name = descr.name; > this.instanceDir = descr.instanceDir; > this.configName = descr.configName; > this.schemaName = descr.schemaName; > + this.name = descr.name; > coreContainer = descr.coreContainer; > } > > @@ -72,17 +67,7 @@ > public String getDefaultDataDir() { > return this.instanceDir + "data/"; > } > - > - /[EMAIL PROTECTED] the core name. */ > - public String getName() { > - return name; > - } > - > - /** Sets the core name. */ > - public void setName(String name) { > - this.name = name; > - } > - > + > /[EMAIL PROTECTED] the core instance directory. */ > public String getInstanceDir() { > return instanceDir; > @@ -111,16 +96,13 @@ > public String getSchemaName() { > return this.schemaName; > } > - > - public SolrCore getCore() { > - return core; > - } > - > - public void setCore(SolrCore core) { > - this.core = core; > + > + /[EMAIL PROTECTED] the initial core name */ > + public String getName() { > + return this.name; > } > > - public CoreContainer getMultiCore() { > + public CoreContainer getCoreContainer() { > return coreContainer; > } > } > > Modified: lucene/solr/trunk/src/java/org/apache/solr/core/SolrCore.java > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/apache/solr/core/SolrCore.java?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- lucene/solr/trunk/src/java/org/apache/solr/core/SolrCore.java (original) > +++ lucene/solr/trunk/src/java/org/apache/solr/core/SolrCore.java Mon Aug 18 > 08:08:28 2008 > @@ -62,13 +62,15 @@ > import java.io.IOException; > import java.util.*; > import java.util.concurrent.*; > +import java.util.concurrent.atomic.AtomicInteger; > import java.util.logging.Logger; > +import java.net.URL; > > > /** > * @version $Id$ > */ > -public final class SolrCore { > +public final class SolrCore implements SolrInfoMBean { > public static final String version="1.0"; > > public static Logger log = Logger.getLogger(SolrCore.class.getName()); > @@ -374,7 +376,7 @@ > * @since solr 1.0 > */ > public SolrCore(String dataDir, IndexSchema schema) throws > ParserConfigurationException, IOException, SAXException { > - this( null, dataDir, new SolrConfig(), schema, null ); > + this(null, dataDir, new SolrConfig(), schema, null ); > } > > /** > @@ -479,8 +481,9 @@ > latch.countDown(); > } > } // end synchronized > + > + infoRegistry.put("core", this); > } > - > > > /** > @@ -569,16 +572,39 @@ > } > return chain; > } > + > + // this core current usage count > + private final AtomicInteger refCount = new AtomicInteger(1); > > + final void open() { > + refCount.incrementAndGet(); > + } > + > /** > * Close all resources allocated by the core. > * 1. searcher > * 2. updateHandler > * 3. all CloseHooks will be notified > * 4. All MBeans will be unregistered from MBeanServer if JMX was enabled > + * <p> > + * This should always be called when the core is obtained through: > + * @see CoreContainer.getCore > + * @see CoreContainer.getAdminCore > + * </p> > + * 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 [EMAIL PROTECTED]"); > + return; > + } > + log.info(logid+" CLOSING SolrCore " + this); > try { > infoRegistry.clear(); > } catch (Exception e) { > @@ -602,16 +628,22 @@ > if( closeHooks != null ) { > for( CloseHook hook : closeHooks ) { > hook.close( this ); > - } > + } > } > } > > + /** Current core usage count. */ > + public int getOpenCount() { > + return refCount.get(); > + } > + > + /** Whether this core is closed. */ > public boolean isClosed() { > - return _searcher == null; > + return refCount.get() <= 0; > } > > - @Override > - protected void finalize() { close(); } > + // this can cause an extra close > + // protected void finalize() { close(); } > > private List<CloseHook> closeHooks = null; > > @@ -1380,6 +1412,45 @@ > 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; > + } > + > } > > > > Modified: > lucene/solr/trunk/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- > lucene/solr/trunk/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java > (original) > +++ > lucene/solr/trunk/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java > Mon Aug 18 08:08:28 2008 > @@ -90,9 +90,8 @@ > > 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 @@ > 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 @@ > case STATUS: { > NamedList<Object> status = new SimpleOrderedMap<Object>(); > 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 @@ > info.add("index", > LukeRequestHandler.getIndexInfo(searcher.get().getReader(), false)); > searcher.decref(); > } finally { > - // solr-647 > - // core.close(); > + core.close(); > } > } > return info; > > Modified: lucene/solr/trunk/src/java/org/apache/solr/util/TestHarness.java > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/src/java/org/apache/solr/util/TestHarness.java?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- lucene/solr/trunk/src/java/org/apache/solr/util/TestHarness.java > (original) > +++ lucene/solr/trunk/src/java/org/apache/solr/util/TestHarness.java Mon Aug > 18 08:08:28 2008 > @@ -21,6 +21,9 @@ > 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 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 @@ > * @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 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 @@ > 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 @@ > > 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 @@ > * 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; > + } > } > > /** > > Modified: lucene/solr/trunk/src/test/org/apache/solr/core/SolrCoreTest.java > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/src/test/org/apache/solr/core/SolrCoreTest.java?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- lucene/solr/trunk/src/test/org/apache/solr/core/SolrCoreTest.java > (original) > +++ lucene/solr/trunk/src/test/org/apache/solr/core/SolrCoreTest.java Mon Aug > 18 08:08:28 2008 > @@ -24,13 +24,14 @@ > 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 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 @@ > 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<Callable<Integer>> callees = new ArrayList<Callable<Integer>>(MT); > + final CoreContainer cores = h.getCoreContainer(); > + for (int i = 0; i < MT; ++i) { > + Callable<Integer> call = new Callable<Integer>() { > + 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<Future<Integer>> results = service.invokeAll(callees); > + for (Future<Integer> 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; > > > Modified: > lucene/solr/trunk/src/webapp/src/org/apache/solr/servlet/SolrDispatchFilter.java > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/src/webapp/src/org/apache/solr/servlet/SolrDispatchFilter.java?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- > lucene/solr/trunk/src/webapp/src/org/apache/solr/servlet/SolrDispatchFilter.java > (original) > +++ > lucene/solr/trunk/src/webapp/src/org/apache/solr/servlet/SolrDispatchFilter.java > Mon Aug 18 08:08:28 2008 > @@ -285,6 +285,9 @@ > if( solrReq != null ) { > solrReq.close(); > } > + if (core != null) { > + core.close(); > + } > } > } > > > Modified: lucene/solr/trunk/src/webapp/web/admin/index.jsp > URL: > http://svn.apache.org/viewvc/lucene/solr/trunk/src/webapp/web/admin/index.jsp?rev=686780&r1=686779&r2=686780&view=diff > ============================================================================== > --- lucene/solr/trunk/src/webapp/web/admin/index.jsp (original) > +++ lucene/solr/trunk/src/webapp/web/admin/index.jsp Mon Aug 18 08:08:28 2008 > @@ -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 @@ > </tr> > > <%-- 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<SolrCore> cores = multicore.getCores(); > -if (cores.size() > 1) {%><tr><td><strong>Cores:</strong><br></td><td><% > - java.util.Iterator<SolrCore> icore = cores.iterator(); > - while (icore.hasNext()) { > - SolrCore acore = icore.next(); > - if (acore == core) continue; > - %>[<a > href="../../<%=acore.getName()%>/admin/"><%=acore.getName()%></a>]<% > +<% org.apache.solr.core.CoreContainer cores = > (org.apache.solr.core.CoreContainer)request.getAttribute("org.apache.solr.CoreContainer"); > + if (cores!=null) { > + Collection<String> names = cores.getCoreNames(); > + if (names.size() > 1) {%><tr><td><strong>Cores:</strong><br></td><td><% > + for (String name : names) { > + %>[<a href="../../<%=name%>/admin/"><%=name%></a>]<% > }%></td></tr><% > }}%> > > > >
-- --Noble Paul