FYI, I filed https://issues.apache.org/jira/browse/SOLR-4505
- Mark On Feb 26, 2013, at 12:16 AM, Mark Miller <[email protected]> wrote: > You are finding great stuff with this stress test driver! We really need this > in the tests :) > > On Feb 25, 2013, at 8:25 AM, Erick Erickson <[email protected]> wrote: > >> What do you think about me checking 4196 in and opening a separate JIRA for >> this issue? > > I think that is fine - I don't think the deadlock is related to 4196. > > I think the following patch will address it: > > Index: solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java > =================================================================== > --- solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java > (revision 1450011) > +++ solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java > (working copy) > @@ -348,10 +348,13 @@ > > // currently for testing purposes. Do a delete of complete index w/o > worrying about versions, don't log, clean up most state in update log, etc > if (delAll && cmd.getVersion() == -Long.MAX_VALUE) { > - synchronized (solrCoreState.getUpdateLock()) { > + solrCoreState.getUpdateLock().lock(); > + try { > deleteAll(); > ulog.deleteAll(); > return; > + } finally { > + solrCoreState.getUpdateLock().unlock(); > } > } > > @@ -361,7 +364,8 @@ > // a realtime view of the index. When a new searcher is opened after a > DBQ, that > // flag can be cleared. If those thing happen concurrently, it's not > thread safe. > // > - synchronized (solrCoreState.getUpdateLock()) { > + solrCoreState.getUpdateLock().lock(); > + try { > if (delAll) { > deleteAll(); > } else { > @@ -374,6 +378,8 @@ > } > > if (ulog != null) ulog.deleteByQuery(cmd); > + } finally { > + solrCoreState.getUpdateLock().unlock(); > } > > madeIt = true; > @@ -397,7 +403,8 @@ > Term idTerm = new Term(idField.getName(), cmd.getIndexedId()); > > // see comment in deleteByQuery > - synchronized (solrCoreState.getUpdateLock()) { > + solrCoreState.getUpdateLock().lock(); > + try { > RefCounted<IndexWriter> iw = solrCoreState.getIndexWriter(core); > try { > IndexWriter writer = iw.get(); > @@ -412,6 +419,8 @@ > } > > if (ulog != null) ulog.add(cmd, true); > + } finally { > + solrCoreState.getUpdateLock().unlock(); > } > > } > @@ -527,10 +536,13 @@ > } > > if (!cmd.softCommit) { > - synchronized (solrCoreState.getUpdateLock()) { // sync is > currently needed to prevent preCommit > - // from being called between preSoft and > - // postSoft... see postSoft comments. > + solrCoreState.getUpdateLock().lock(); // sync is currently needed > to prevent preCommit > + // from being called between > preSoft and > + // postSoft... see postSoft > comments. > + try { > if (ulog != null) ulog.preCommit(cmd); > + } finally { > + solrCoreState.getUpdateLock().unlock(); > } > > // SolrCore.verbose("writer.commit() start writer=",writer); > @@ -557,14 +569,18 @@ > > if (cmd.softCommit) { > // ulog.preSoftCommit(); > - synchronized (solrCoreState.getUpdateLock()) { > + solrCoreState.getUpdateLock().lock(); > + try { > if (ulog != null) ulog.preSoftCommit(cmd); > core.getSearcher(true, false, waitSearcher, true); > if (ulog != null) ulog.postSoftCommit(cmd); > + } finally { > + solrCoreState.getUpdateLock().unlock(); > } > // ulog.postSoftCommit(); > } else { > - synchronized (solrCoreState.getUpdateLock()) { > + solrCoreState.getUpdateLock().lock(); > + try { > if (ulog != null) ulog.preSoftCommit(cmd); > if (cmd.openSearcher) { > core.getSearcher(true, false, waitSearcher); > @@ -574,6 +590,8 @@ > searchHolder.decref(); > } > if (ulog != null) ulog.postSoftCommit(cmd); > + } finally { > + solrCoreState.getUpdateLock().unlock(); > } > if (ulog != null) ulog.postCommit(cmd); // postCommit currently means > new searcher has > // also been opened > @@ -713,8 +731,11 @@ > // TODO: keep other commit callbacks from being called? > // this.commit(cmd); // too many test failures using this > method... is it because of callbacks? > > - synchronized (solrCoreState.getUpdateLock()) { > + solrCoreState.getUpdateLock().lock(); > + try { > ulog.preCommit(cmd); > + } finally { > + solrCoreState.getUpdateLock().unlock(); > } > > // todo: refactor this shared code (or figure out why a real > CommitUpdateCommand can't be used) > @@ -723,8 +744,11 @@ > writer.setCommitData(commitData); > writer.commit(); > > - synchronized (solrCoreState.getUpdateLock()) { > + solrCoreState.getUpdateLock().lock(); > + try { > ulog.postCommit(cmd); > + } finally { > + solrCoreState.getUpdateLock().unlock(); > } > } > } catch (Throwable th) { > Index: solr/core/src/java/org/apache/solr/update/SolrCoreState.java > =================================================================== > --- solr/core/src/java/org/apache/solr/update/SolrCoreState.java > (revision 1450012) > +++ solr/core/src/java/org/apache/solr/update/SolrCoreState.java > (working copy) > @@ -37,12 +37,6 @@ > public abstract class SolrCoreState { > public static Logger log = LoggerFactory.getLogger(SolrCoreState.class); > > - private final Object deleteLock = new Object(); > - > - public Object getUpdateLock() { > - return deleteLock; > - } > - > private int solrCoreStateRefCnt = 1; > > public synchronized int getSolrCoreStateRefCnt() { > @@ -76,6 +70,7 @@ > } > > public abstract Lock getCommitLock(); > + public abstract Lock getUpdateLock(); > > /** > * Force the creation of a new IndexWriter using the settings from the given > Index: solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java > =================================================================== > --- solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java > (revision 1450012) > +++ solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java > (working copy) > @@ -56,6 +56,7 @@ > private boolean writerFree = true; > > protected final ReentrantLock commitLock = new ReentrantLock(); > + protected final ReentrantLock updateLock = new ReentrantLock(); > > public DefaultSolrCoreState(DirectoryFactory directoryFactory) { > this.directoryFactory = directoryFactory; > @@ -142,11 +143,16 @@ > log.info("Waiting until IndexWriter is unused... core=" + coreName); > > boolean yieldedCommitLock = false; > + boolean yieldedUpdateLock = false; > try { > if (commitLock.isHeldByCurrentThread()) { > yieldedCommitLock = true; > commitLock.unlock(); > } > + if (updateLock.isHeldByCurrentThread()) { > + yieldedUpdateLock = true; > + updateLock.unlock(); > + } > > while (!writerFree) { > try { > @@ -161,6 +167,9 @@ > if (yieldedCommitLock) { > commitLock.lock(); > } > + if (yieldedUpdateLock) { > + updateLock.lock(); > + } > } > > try { > @@ -295,4 +304,9 @@ > return commitLock; > } > > + @Override > + public Lock getUpdateLock() { > + return updateLock; > + } > + > } > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
