You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by er...@apache.org on 2015/08/09 07:46:43 UTC
svn commit: r1694855 - in /lucene/dev/branches/branch_5x: ./ solr/
solr/core/ solr/core/src/java/org/apache/solr/update/
solr/core/src/test/org/apache/solr/search/
Author: erick
Date: Sun Aug 9 05:46:42 2015
New Revision: 1694855
URL: http://svn.apache.org/r1694855
Log:
SOLR-7836: Possible deadlock when closing refcounted index writers
Added:
lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/search/TestReloadDeadlock.java
- copied unchanged from r1694854, lucene/dev/trunk/solr/core/src/test/org/apache/solr/search/TestReloadDeadlock.java
Modified:
lucene/dev/branches/branch_5x/ (props changed)
lucene/dev/branches/branch_5x/solr/ (props changed)
lucene/dev/branches/branch_5x/solr/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_5x/solr/core/ (props changed)
lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java
lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
Modified: lucene/dev/branches/branch_5x/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/CHANGES.txt?rev=1694855&r1=1694854&r2=1694855&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/solr/CHANGES.txt Sun Aug 9 05:46:42 2015
@@ -39,6 +39,9 @@ New Features
Bug Fixes
----------------------
+* SOLR-7836: Possible deadlock when closing refcounted index writers.
+ (Jessica Cheng Mallet, Erick Erickson)
+
Optimizations
----------------------
Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java?rev=1694855&r1=1694854&r2=1694855&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java Sun Aug 9 05:46:42 2015
@@ -100,27 +100,24 @@ public final class DefaultSolrCoreState
throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE, "Already closed");
}
}
-
+
if (core == null) {
// core == null is a signal to just return the current writer, or null
// if none.
initRefCntWriter();
if (refCntWriter == null) return null;
- writerFree = false;
- writerPauseLock.notifyAll();
- if (refCntWriter != null) refCntWriter.incref();
-
- return refCntWriter;
- }
-
- if (indexWriter == null) {
- indexWriter = createMainIndexWriter(core, "DirectUpdateHandler2");
+ } else {
+ if (indexWriter == null) {
+ indexWriter = createMainIndexWriter(core, "DirectUpdateHandler2");
+ }
+ initRefCntWriter();
}
- initRefCntWriter();
+
writerFree = false;
- writerPauseLock.notifyAll();
refCntWriter.incref();
+ writerPauseLock.notifyAll();
return refCntWriter;
+
}
}
@@ -152,18 +149,18 @@ public final class DefaultSolrCoreState
pauseWriter = true;
// then lets wait until it's out of use
log.info("Waiting until IndexWriter is unused... core=" + coreName);
-
- while (!writerFree) {
- try {
- writerPauseLock.wait(100);
- } catch (InterruptedException e) {}
-
- if (closed) {
- throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE, "SolrCoreState already closed");
- }
- }
try {
+ while (!writerFree) {
+ try {
+ writerPauseLock.wait(100);
+ } catch (InterruptedException e) {
+ }
+ if (closed) {
+ throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE, "SolrCoreState already closed");
+ }
+ }
+
if (indexWriter != null) {
if (!rollback) {
try {
@@ -188,7 +185,6 @@ public final class DefaultSolrCoreState
// we need to null this so it picks up the new writer next get call
refCntWriter = null;
} finally {
-
pauseWriter = false;
writerPauseLock.notifyAll();
}
@@ -210,39 +206,44 @@ public final class DefaultSolrCoreState
pauseWriter = true;
// then lets wait until it's out of use
log.info("Waiting until IndexWriter is unused... core=" + coreName);
-
- while (!writerFree) {
- try {
- writerPauseLock.wait(100);
- } catch (InterruptedException e) {}
-
- if (closed) {
- throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE,
- "SolrCoreState already closed");
- }
- }
-
- if (indexWriter != null) {
- if (!rollback) {
+ try {
+ while (!writerFree) {
try {
- log.info("Closing old IndexWriter... core=" + coreName);
- indexWriter.close();
- } catch (Exception e) {
- SolrException.log(log, "Error closing old IndexWriter. core="
- + coreName, e);
+ writerPauseLock.wait(100);
+ } catch (InterruptedException e) {
}
- } else {
- try {
- log.info("Rollback old IndexWriter... core=" + coreName);
- indexWriter.rollback();
- } catch (Exception e) {
- SolrException.log(log, "Error rolling back old IndexWriter. core="
- + coreName, e);
+
+ if (closed) {
+ throw new SolrException(ErrorCode.SERVICE_UNAVAILABLE,
+ "SolrCoreState already closed");
+ }
+ }
+
+ if (indexWriter != null) {
+ if (!rollback) {
+ try {
+ log.info("Closing old IndexWriter... core=" + coreName);
+ indexWriter.close();
+ } catch (Exception e) {
+ SolrException.log(log, "Error closing old IndexWriter. core="
+ + coreName, e);
+ }
+ } else {
+ try {
+ log.info("Rollback old IndexWriter... core=" + coreName);
+ indexWriter.rollback();
+ } catch (Exception e) {
+ SolrException.log(log, "Error rolling back old IndexWriter. core="
+ + coreName, e);
+ }
}
}
+ } finally {
+ pauseWriter = false;
+ writerPauseLock.notifyAll();
}
-
}
+
}
@Override
Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java?rev=1694855&r1=1694854&r2=1694855&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java Sun Aug 9 05:46:42 2015
@@ -178,124 +178,154 @@ public class DirectUpdateHandler2 extend
*/
private int addDoc0(AddUpdateCommand cmd) throws IOException {
int rc = -1;
+
+ addCommands.incrementAndGet();
+ addCommandsCumulative.incrementAndGet();
+
+ // if there is no ID field, don't overwrite
+ if (idField == null) {
+ cmd.overwrite = false;
+ }
+ try {
+ if (cmd.overwrite) {
+ // Check for delete by query commands newer (i.e. reordered). This
+ // should always be null on a leader
+ List<UpdateLog.DBQ> deletesAfter = null;
+ if (ulog != null && cmd.version > 0) {
+ deletesAfter = ulog.getDBQNewer(cmd.version);
+ }
+
+ if (deletesAfter != null) {
+ addAndDelete(cmd, deletesAfter);
+ } else {
+ doNormalUpdate(cmd);
+ }
+ } else {
+ allowDuplicateUpdate(cmd);
+ }
+
+ if ((cmd.getFlags() & UpdateCommand.IGNORE_AUTOCOMMIT) == 0) {
+ if (commitWithinSoftCommit) {
+ commitTracker.addedDocument(-1);
+ softCommitTracker.addedDocument(cmd.commitWithin);
+ } else {
+ softCommitTracker.addedDocument(-1);
+ commitTracker.addedDocument(cmd.commitWithin);
+ }
+ }
+
+ rc = 1;
+ } finally {
+ if (rc != 1) {
+ numErrors.incrementAndGet();
+ numErrorsCumulative.incrementAndGet();
+ } else {
+ numDocsPending.incrementAndGet();
+ }
+ }
+
+ return rc;
+ }
+
+ private void allowDuplicateUpdate(AddUpdateCommand cmd) throws IOException {
RefCounted<IndexWriter> iw = solrCoreState.getIndexWriter(core);
try {
IndexWriter writer = iw.get();
- addCommands.incrementAndGet();
- addCommandsCumulative.incrementAndGet();
-
- // if there is no ID field, don't overwrite
- if (idField == null) {
- cmd.overwrite = false;
+
+ if (cmd.isBlock()) {
+ writer.addDocuments(cmd);
+ } else {
+ writer.addDocument(cmd.getLuceneDocument());
}
-
+ } finally {
+ iw.decref();
+ }
+
+ if (ulog != null) ulog.add(cmd);
+ }
+
+ private void doNormalUpdate(AddUpdateCommand cmd) throws IOException {
+ Term updateTerm;
+ Term idTerm = new Term(cmd.isBlock() ? "_root_" : idField.getName(), cmd.getIndexedId());
+ boolean del = false;
+ if (cmd.updateTerm == null) {
+ updateTerm = idTerm;
+ } else {
+ // this is only used by the dedup update processor
+ del = true;
+ updateTerm = cmd.updateTerm;
+ }
+
+ RefCounted<IndexWriter> iw = solrCoreState.getIndexWriter(core);
+ try {
+ IndexWriter writer = iw.get();
+
+ if (cmd.isBlock()) {
+ writer.updateDocuments(updateTerm, cmd);
+ } else {
+ Document luceneDocument = cmd.getLuceneDocument();
+ // SolrCore.verbose("updateDocument",updateTerm,luceneDocument,writer);
+ writer.updateDocument(updateTerm, luceneDocument);
+ }
+ // SolrCore.verbose("updateDocument",updateTerm,"DONE");
+
+ if (del) { // ensure id remains unique
+ BooleanQuery.Builder bq = new BooleanQuery.Builder();
+ bq.add(new BooleanClause(new TermQuery(updateTerm),
+ Occur.MUST_NOT));
+ bq.add(new BooleanClause(new TermQuery(idTerm), Occur.MUST));
+ writer.deleteDocuments(new DeleteByQueryWrapper(bq.build(), core.getLatestSchema()));
+ }
+ } finally {
+ iw.decref();
+ }
+
+ // Add to the transaction log *after* successfully adding to the
+ // index, if there was no error.
+ // This ordering ensures that if we log it, it's definitely been
+ // added to the the index.
+ // This also ensures that if a commit sneaks in-between, that we
+ // know everything in a particular
+ // log version was definitely committed.
+ if (ulog != null) ulog.add(cmd);
+ }
+
+ private void addAndDelete(AddUpdateCommand cmd, List<UpdateLog.DBQ> deletesAfter) throws IOException {
+
+ log.info("Reordered DBQs detected. Update=" + cmd + " DBQs="
+ + deletesAfter);
+ List<Query> dbqList = new ArrayList<>(deletesAfter.size());
+ for (UpdateLog.DBQ dbq : deletesAfter) {
try {
- IndexSchema schema = cmd.getReq().getSchema();
-
- if (cmd.overwrite) {
-
- // Check for delete by query commands newer (i.e. reordered). This
- // should always be null on a leader
- List<UpdateLog.DBQ> deletesAfter = null;
- if (ulog != null && cmd.version > 0) {
- deletesAfter = ulog.getDBQNewer(cmd.version);
- }
-
- if (deletesAfter != null) {
- log.info("Reordered DBQs detected. Update=" + cmd + " DBQs="
- + deletesAfter);
- List<Query> dbqList = new ArrayList<>(deletesAfter.size());
- for (UpdateLog.DBQ dbq : deletesAfter) {
- try {
- DeleteUpdateCommand tmpDel = new DeleteUpdateCommand(cmd.req);
- tmpDel.query = dbq.q;
- tmpDel.version = -dbq.version;
- dbqList.add(getQuery(tmpDel));
- } catch (Exception e) {
- log.error("Exception parsing reordered query : " + dbq, e);
- }
- }
-
- addAndDelete(cmd, dbqList);
- } else {
- // normal update
-
- Term updateTerm;
- Term idTerm = new Term(cmd.isBlock() ? "_root_" : idField.getName(), cmd.getIndexedId());
- boolean del = false;
- if (cmd.updateTerm == null) {
- updateTerm = idTerm;
- } else {
- // this is only used by the dedup update processor
- del = true;
- updateTerm = cmd.updateTerm;
- }
-
- if (cmd.isBlock()) {
- writer.updateDocuments(updateTerm, cmd);
- } else {
- Document luceneDocument = cmd.getLuceneDocument();
- // SolrCore.verbose("updateDocument",updateTerm,luceneDocument,writer);
- writer.updateDocument(updateTerm, luceneDocument);
- }
- // SolrCore.verbose("updateDocument",updateTerm,"DONE");
-
- if (del) { // ensure id remains unique
- BooleanQuery.Builder bq = new BooleanQuery.Builder();
- bq.add(new BooleanClause(new TermQuery(updateTerm),
- Occur.MUST_NOT));
- bq.add(new BooleanClause(new TermQuery(idTerm), Occur.MUST));
- writer.deleteDocuments(new DeleteByQueryWrapper(bq.build(), core.getLatestSchema()));
- }
-
- // Add to the transaction log *after* successfully adding to the
- // index, if there was no error.
- // This ordering ensures that if we log it, it's definitely been
- // added to the the index.
- // This also ensures that if a commit sneaks in-between, that we
- // know everything in a particular
- // log version was definitely committed.
- if (ulog != null) ulog.add(cmd);
- }
-
- } else {
- // allow duplicates
- if (cmd.isBlock()) {
- writer.addDocuments(cmd);
- } else {
- writer.addDocument(cmd.getLuceneDocument());
- }
+ DeleteUpdateCommand tmpDel = new DeleteUpdateCommand(cmd.req);
+ tmpDel.query = dbq.q;
+ tmpDel.version = -dbq.version;
+ dbqList.add(getQuery(tmpDel));
+ } catch (Exception e) {
+ log.error("Exception parsing reordered query : " + dbq, e);
+ }
+ }
- if (ulog != null) ulog.add(cmd);
- }
-
- if ((cmd.getFlags() & UpdateCommand.IGNORE_AUTOCOMMIT) == 0) {
- if (commitWithinSoftCommit) {
- commitTracker.addedDocument(-1);
- softCommitTracker.addedDocument(cmd.commitWithin);
- } else {
- softCommitTracker.addedDocument(-1);
- commitTracker.addedDocument(cmd.commitWithin);
- }
- }
-
- rc = 1;
- } finally {
- if (rc != 1) {
- numErrors.incrementAndGet();
- numErrorsCumulative.incrementAndGet();
- } else {
- numDocsPending.incrementAndGet();
+ Document luceneDocument = cmd.getLuceneDocument();
+ Term idTerm = new Term(idField.getName(), cmd.getIndexedId());
+
+ RefCounted<IndexWriter> iw = solrCoreState.getIndexWriter(core);
+ try {
+ IndexWriter writer = iw.get();
+
+ // see comment in deleteByQuery
+ synchronized (solrCoreState.getUpdateLock()) {
+ writer.updateDocument(idTerm, luceneDocument);
+ for (Query q : dbqList) {
+ writer.deleteDocuments(new DeleteByQueryWrapper(q, core.getLatestSchema()));
}
}
-
} finally {
iw.decref();
}
-
- return rc;
+ if (ulog != null) ulog.add(cmd, true);
}
-
+
private void updateDeleteTrackers(DeleteUpdateCommand cmd) {
if ((cmd.getFlags() & UpdateCommand.IGNORE_AUTOCOMMIT) == 0) {
if (commitWithinSoftCommit) {
@@ -427,35 +457,6 @@ public class DirectUpdateHandler2 extend
}
-
- /** Add a document execute the deletes as atomically as possible */
- private void addAndDelete(AddUpdateCommand cmd, List<Query> dbqList)
- throws IOException {
- Document luceneDocument = cmd.getLuceneDocument();
- Term idTerm = new Term(idField.getName(), cmd.getIndexedId());
-
- // see comment in deleteByQuery
- synchronized (solrCoreState.getUpdateLock()) {
- RefCounted<IndexWriter> iw = solrCoreState.getIndexWriter(core);
- try {
- IndexWriter writer = iw.get();
- writer.updateDocument(idTerm, luceneDocument);
-
- for (Query q : dbqList) {
- writer.deleteDocuments(new DeleteByQueryWrapper(q, core.getLatestSchema()));
- }
- } finally {
- iw.decref();
- }
-
- if (ulog != null) ulog.add(cmd, true);
- }
-
- }
-
-
-
-
@Override
public int mergeIndexes(MergeIndexesCommand cmd) throws IOException {
mergeIndexesCommands.incrementAndGet();