Author: epbernard Date: 2008-09-08 10:07:27 -0400 (Mon, 08 Sep 2008) New Revision: 15156 Added: search/trunk/src/test/org/hibernate/search/test/reader/SharedBufferedReaderPerfTest.java Modified: search/trunk/doc/reference/en/modules/query.xml search/trunk/src/java/org/hibernate/search/backend/OptimizeLuceneWork.java search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java search/trunk/src/java/org/hibernate/search/reader/NotSharedReaderProvider.java search/trunk/src/java/org/hibernate/search/reader/ReaderProvider.java search/trunk/src/java/org/hibernate/search/reader/SharedReaderProvider.java search/trunk/src/java/org/hibernate/search/reader/SharingBufferReaderProvider.java search/trunk/src/test/org/hibernate/search/test/reader/ReaderPerfTestCase.java Log: HSEARCH-235 add ReaderProvider.destroy() called during SearchFactory.close() Modified: search/trunk/doc/reference/en/modules/query.xml =================================================================== --- search/trunk/doc/reference/en/modules/query.xml 2008-09-05 15:26:25 UTC (rev 15155) +++ search/trunk/doc/reference/en/modules/query.xml 2008-09-08 14:07:27 UTC (rev 15156) @@ -606,7 +606,7 @@ <literal>hibernate.search.filter.cache_strategy</literal>.</para> <para>The described filter cache mechanism should not be confused with - caching the actual filter results. In Lucene it is common practise to wrap + caching the actual filter results. In Lucene it is common practice to wrap filters using the <classname>IndexReader</classname> around a <classname>CachingWrapperFilter.</classname> The wrapper will cache the <classname>BitSet</classname> returned from the Modified: search/trunk/src/java/org/hibernate/search/backend/OptimizeLuceneWork.java =================================================================== --- search/trunk/src/java/org/hibernate/search/backend/OptimizeLuceneWork.java 2008-09-05 15:26:25 UTC (rev 15155) +++ search/trunk/src/java/org/hibernate/search/backend/OptimizeLuceneWork.java 2008-09-08 14:07:27 UTC (rev 15156) @@ -4,6 +4,10 @@ import java.io.Serializable; /** + * A unit of work triggering an optimize operation + * This work does not propagate to a cluster: it should be filtered before being sent to + * the network + * * @author Andrew Hahn * @author Emmanuel Bernard */ Modified: search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java =================================================================== --- search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java 2008-09-05 15:26:25 UTC (rev 15155) +++ search/trunk/src/java/org/hibernate/search/impl/SearchFactoryImpl.java 2008-09-08 14:07:27 UTC (rev 15156) @@ -147,6 +147,14 @@ catch (Exception e) { log.error( "Worker raises an exception on close()", e ); } + + try { + readerProvider.destroy(); + } + catch (Exception e) { + log.error( "ReaderProvider raises an exception on destroy()", e ); + } + //TODO move to DirectoryProviderFactory for cleaner for (DirectoryProvider dp : getDirectoryProviders() ) { try { Modified: search/trunk/src/java/org/hibernate/search/reader/NotSharedReaderProvider.java =================================================================== --- search/trunk/src/java/org/hibernate/search/reader/NotSharedReaderProvider.java 2008-09-05 15:26:25 UTC (rev 15155) +++ search/trunk/src/java/org/hibernate/search/reader/NotSharedReaderProvider.java 2008-09-08 14:07:27 UTC (rev 15156) @@ -47,4 +47,7 @@ public void initialize(Properties props, SearchFactoryImplementor searchFactoryImplementor) { } + + public void destroy() { + } } Modified: search/trunk/src/java/org/hibernate/search/reader/ReaderProvider.java =================================================================== --- search/trunk/src/java/org/hibernate/search/reader/ReaderProvider.java 2008-09-05 15:26:25 UTC (rev 15155) +++ search/trunk/src/java/org/hibernate/search/reader/ReaderProvider.java 2008-09-08 14:07:27 UTC (rev 15156) @@ -30,5 +30,14 @@ */ void closeReader(IndexReader reader); + /** + * inialize the reader provider before its use + */ void initialize(Properties props, SearchFactoryImplementor searchFactoryImplementor); + + /** + * called when a SearchFactory is destroyed. This method typically releases resources + * This method is guaranteed to be executed after readers are released by queries (assuming no user error). + */ + void destroy(); } Modified: search/trunk/src/java/org/hibernate/search/reader/SharedReaderProvider.java =================================================================== --- search/trunk/src/java/org/hibernate/search/reader/SharedReaderProvider.java 2008-09-05 15:26:25 UTC (rev 15155) +++ search/trunk/src/java/org/hibernate/search/reader/SharedReaderProvider.java 2008-09-08 14:07:27 UTC (rev 15156) @@ -40,7 +40,7 @@ */ private Map<DirectoryProvider, Lock> perDirectoryProviderManipulationLocks; /** - * Contain the active (ie non obsolete IndexReader for a given Directory + * Contains the active (ie non obsolete IndexReader for a given Directory * There may be no entry (warm up) * <p/> * protected by semaphoreIndexReaderLock @@ -217,67 +217,76 @@ } for (IndexReader subReader : readers) { - ReaderData readerData; - //TODO can we avoid that lock? + closeInternalReader( trace, subReader, false ); + } + } + + private void closeInternalReader(boolean trace, IndexReader subReader, boolean finalClose) { + ReaderData readerData; + //TODO can we avoid that lock? + semaphoreIndexReaderLock.lock(); + try { + readerData = searchIndexReaderSemaphores.get( subReader ); + } + finally { + semaphoreIndexReaderLock.unlock(); + } + + if ( readerData == null ) { + log.error( "Trying to close a Lucene IndexReader not present: {}", subReader.directory() ); + //TODO should we try to close? + return; + } + + //acquire the locks in the same order as everywhere else + Lock directoryProviderLock = perDirectoryProviderManipulationLocks.get( readerData.provider ); + boolean closeReader = false; + directoryProviderLock.lock(); + try { + boolean isActive; + isActive = activeSearchIndexReaders.get( readerData.provider ) == subReader; + if ( trace ) log.trace( "Indexreader not active: {}", subReader ); semaphoreIndexReaderLock.lock(); try { readerData = searchIndexReaderSemaphores.get( subReader ); - } - finally { - semaphoreIndexReaderLock.unlock(); - } + if ( readerData == null ) { + log.error( "Trying to close a Lucene IndexReader not present: {}" + subReader.directory() ); + //TODO should we try to close? + return; + } - if ( readerData == null ) { - log.error( "Trying to close a Lucene IndexReader not present: {}", subReader.directory() ); - //TODO should we try to close? - continue; - } - - //acquire the locks in the same order as everywhere else - Lock directoryProviderLock = perDirectoryProviderManipulationLocks.get( readerData.provider ); - boolean closeReader = false; - directoryProviderLock.lock(); - try { - boolean isActive; - isActive = activeSearchIndexReaders.get( readerData.provider ) == subReader; - if ( trace ) log.trace( "Indexreader not active: {}", subReader ); - semaphoreIndexReaderLock.lock(); - try { - readerData = searchIndexReaderSemaphores.get( subReader ); - if ( readerData == null ) { - log.error( "Trying to close a Lucene IndexReader not present: {}" + subReader.directory() ); - //TODO should we try to close? - continue; - } + //final close, the semaphore should be at 0 already + if (!finalClose) { readerData.semaphore--; if ( trace ) log.trace( "Semaphore decreased to: {} for {}", readerData.semaphore, subReader ); - if ( readerData.semaphore < 0 ) - log.error( "Semaphore negative: {}", subReader.directory() ); - if ( ( !isActive ) && readerData.semaphore == 0 ) { - searchIndexReaderSemaphores.remove( subReader ); - closeReader = true; - } - else { - closeReader = false; - } } - finally { - semaphoreIndexReaderLock.unlock(); + + if ( readerData.semaphore < 0 ) + log.error( "Semaphore negative: {}", subReader.directory() ); + if ( ( !isActive ) && readerData.semaphore == 0 ) { + searchIndexReaderSemaphores.remove( subReader ); + closeReader = true; } + else { + closeReader = false; + } } finally { - directoryProviderLock.unlock(); + semaphoreIndexReaderLock.unlock(); } + } + finally { + directoryProviderLock.unlock(); + } - if ( closeReader ) { - if ( trace ) log.trace( "Closing IndexReader: {}", subReader ); - try { - subReader.close(); - } - catch (IOException e) { - log.warn( "Unable to close Lucene IndexReader", e ); - } + if ( closeReader ) { + if ( trace ) log.trace( "Closing IndexReader: {}", subReader ); + try { + subReader.close(); } + catch ( IOException e) { + log.warn( "Unable to close Lucene IndexReader", e ); + } } } @@ -290,6 +299,29 @@ perDirectoryProviderManipulationLocks = Collections.unmodifiableMap( perDirectoryProviderManipulationLocks ); } + public void destroy() { + boolean trace = log.isTraceEnabled(); + IndexReader[] readers; + semaphoreIndexReaderLock.lock(); + try { + //release active readers + activeSearchIndexReaders.clear(); + readers = searchIndexReaderSemaphores.keySet().toArray( new IndexReader[searchIndexReaderSemaphores.size()] ); + } + finally { + semaphoreIndexReaderLock.unlock(); + } + + for (IndexReader reader : readers) { + closeInternalReader( trace, reader, true ); + } + + if ( searchIndexReaderSemaphores.size() != 0 ) { + log.warn( "ReaderProvider contains readers not properly closed at destroy time" ); + } + + } + private static class ReaderData { public ReaderData(int semaphore, DirectoryProvider provider) { Modified: search/trunk/src/java/org/hibernate/search/reader/SharingBufferReaderProvider.java =================================================================== --- search/trunk/src/java/org/hibernate/search/reader/SharingBufferReaderProvider.java 2008-09-05 15:26:25 UTC (rev 15155) +++ search/trunk/src/java/org/hibernate/search/reader/SharingBufferReaderProvider.java 2008-09-08 14:07:27 UTC (rev 15156) @@ -76,6 +76,16 @@ currentReaders = Collections.unmodifiableMap( map ); } + public void destroy() { + IndexReader[] readers = allReaders.keySet().toArray( new IndexReader[allReaders.size()] ); + for (IndexReader reader : readers) { + ReaderUsagePair usage = allReaders.get( reader ); + usage.close(); + } + + if ( allReaders.size() != 0 ) log.warn( "ReaderProvider contains readers not properly closed at destroy time" ); + } + public IndexReader openReader(DirectoryProvider... directoryProviders) { int length = directoryProviders.length; IndexReader[] readers = new IndexReader[length]; Modified: search/trunk/src/test/org/hibernate/search/test/reader/ReaderPerfTestCase.java =================================================================== --- search/trunk/src/test/org/hibernate/search/test/reader/ReaderPerfTestCase.java 2008-09-05 15:26:25 UTC (rev 15155) +++ search/trunk/src/test/org/hibernate/search/test/reader/ReaderPerfTestCase.java 2008-09-08 14:07:27 UTC (rev 15156) @@ -50,6 +50,7 @@ protected void tearDown() throws Exception { super.tearDown(); + if ( getSessions() != null ) getSessions().close(); File sub = getBaseIndexDir(); FileHelper.delete( sub ); } Copied: search/trunk/src/test/org/hibernate/search/test/reader/SharedBufferedReaderPerfTest.java (from rev 15153, search/trunk/src/test/org/hibernate/search/test/reader/SharedReaderPerfTest.java) =================================================================== --- search/trunk/src/test/org/hibernate/search/test/reader/SharedBufferedReaderPerfTest.java (rev 0) +++ search/trunk/src/test/org/hibernate/search/test/reader/SharedBufferedReaderPerfTest.java 2008-09-08 14:07:27 UTC (rev 15156) @@ -0,0 +1,15 @@ +//$Id$ +package org.hibernate.search.test.reader; + +import org.hibernate.cfg.Configuration; +import org.hibernate.search.Environment; + +/** + * @author Emmanuel Bernard + */ +public class SharedBufferedReaderPerfTest extends ReaderPerfTestCase { + protected void configure(Configuration cfg) { + super.configure( cfg ); + cfg.setProperty( Environment.READER_STRATEGY, "shared-segments" ); + } +} \ No newline at end of file Property changes on: search/trunk/src/test/org/hibernate/search/test/reader/SharedBufferedReaderPerfTest.java ___________________________________________________________________ Name: svn:keywords + Id Name: svn:mergeinfo +