You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2012/07/30 18:56:10 UTC
svn commit: r1367171 - in /lucene/dev/trunk/solr/core/src:
java/org/apache/solr/handler/ java/org/apache/solr/update/
test/org/apache/solr/cloud/
Author: markrmiller
Date: Mon Jul 30 16:56:09 2012
New Revision: 1367171
URL: http://svn.apache.org/viewvc?rev=1367171&view=rev
Log:
Break out SolrCoreState locks to avoid deadlock
SOLR-3659: on replication, rollback old writer rather than closing it
randomly turn on auto soft commit in monkey test
Modified:
lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/SnapPuller.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/CommitTracker.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrCoreState.java
lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/UpdateHandler.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/ChaosMonkeySafeLeaderTest.java
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java?rev=1367171&r1=1367170&r2=1367171&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java Mon Jul 30 16:56:09 2012
@@ -887,7 +887,7 @@ public class ReplicationHandler extends
}
// reboot the writer on the new index
- core.getUpdateHandler().newIndexWriter();
+ core.getUpdateHandler().newIndexWriter(true);
} catch (IOException e) {
LOG.warn("Unable to get IndexCommit on startup", e);
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/SnapPuller.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/SnapPuller.java?rev=1367171&r1=1367170&r2=1367171&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/SnapPuller.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/SnapPuller.java Mon Jul 30 16:56:09 2012
@@ -535,7 +535,7 @@ public class SnapPuller {
SolrQueryRequest req = new LocalSolrQueryRequest(solrCore,
new ModifiableSolrParams());
// reboot the writer on the new index and get a new searcher
- solrCore.getUpdateHandler().newIndexWriter();
+ solrCore.getUpdateHandler().newIndexWriter(true);
try {
// first try to open an NRT searcher so that the new
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/CommitTracker.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/CommitTracker.java?rev=1367171&r1=1367170&r2=1367171&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/CommitTracker.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/CommitTracker.java Mon Jul 30 16:56:09 2012
@@ -39,8 +39,10 @@ import org.slf4j.LoggerFactory;
* definitely change in the future, so the interface should not be relied-upon
*
* Note: all access must be synchronized.
+ *
+ * Public for tests.
*/
-final class CommitTracker implements Runnable {
+public final class CommitTracker implements Runnable {
protected final static Logger log = LoggerFactory.getLogger(CommitTracker.class);
// scheduler delay for maxDoc-triggered autocommits
@@ -248,7 +250,8 @@ final class CommitTracker implements Run
this.docsUpperBound = docsUpperBound;
}
- void setTimeUpperBound(long timeUpperBound) {
+ // only for testing - not thread safe
+ public void setTimeUpperBound(long timeUpperBound) {
this.timeUpperBound = timeUpperBound;
}
}
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java?rev=1367171&r1=1367170&r2=1367171&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java Mon Jul 30 16:56:09 2012
@@ -35,6 +35,10 @@ public final class DefaultSolrCoreState
private final boolean SKIP_AUTO_RECOVERY = Boolean.getBoolean("solrcloud.skip.autorecovery");
private final Object recoveryLock = new Object();
+
+ // protects pauseWriter and writerFree
+ private final Object writerPauseLock = new Object();
+
private int refCnt = 1;
private SolrIndexWriter indexWriter = null;
private DirectoryFactory directoryFactory;
@@ -55,68 +59,74 @@ public final class DefaultSolrCoreState
@Override
public synchronized RefCounted<IndexWriter> getIndexWriter(SolrCore core)
throws IOException {
-
- if (core == null) {
- // core == null is a signal to just return the current writer, or null if none.
- if (refCntWriter != null) refCntWriter.incref();
- return refCntWriter;
- }
-
- while (pauseWriter) {
- try {
- wait();
- } catch (InterruptedException e) {}
- }
-
- if (indexWriter == null) {
- indexWriter = createMainIndexWriter(core, "DirectUpdateHandler2", false,
- false);
- }
- if (refCntWriter == null) {
- refCntWriter = new RefCounted<IndexWriter>(indexWriter) {
- @Override
- public void close() {
- synchronized (DefaultSolrCoreState.this) {
- writerFree = true;
- DefaultSolrCoreState.this.notifyAll();
+ synchronized (writerPauseLock) {
+ if (core == null) {
+ // core == null is a signal to just return the current writer, or null
+ // if none.
+ if (refCntWriter != null) refCntWriter.incref();
+ return refCntWriter;
+ }
+
+ while (pauseWriter) {
+ try {
+ writerPauseLock.wait();
+ } catch (InterruptedException e) {}
+ }
+
+ if (indexWriter == null) {
+ indexWriter = createMainIndexWriter(core, "DirectUpdateHandler2",
+ false, false);
+ }
+ if (refCntWriter == null) {
+ refCntWriter = new RefCounted<IndexWriter>(indexWriter) {
+ @Override
+ public void close() {
+ synchronized (writerPauseLock) {
+ writerFree = true;
+ writerPauseLock.notifyAll();
+ }
}
- }
- };
+ };
+ }
+ writerFree = false;
+ writerPauseLock.notifyAll();
+ refCntWriter.incref();
+ return refCntWriter;
}
- writerFree = false;
- notifyAll();
- refCntWriter.incref();
- return refCntWriter;
}
@Override
- public synchronized void newIndexWriter(SolrCore core) throws IOException {
- // we need to wait for the Writer to fall out of use
- // first lets stop it from being lent out
- pauseWriter = true;
- // then lets wait until its out of use
- while(!writerFree) {
- try {
- wait();
- } catch (InterruptedException e) {}
- }
+ public synchronized void newIndexWriter(SolrCore core, boolean rollback) throws IOException {
- try {
- if (indexWriter != null) {
+ synchronized (writerPauseLock) {
+ // we need to wait for the Writer to fall out of use
+ // first lets stop it from being lent out
+ pauseWriter = true;
+ // then lets wait until its out of use
+ while (!writerFree) {
try {
- indexWriter.close();
- } catch (Throwable t) {
- SolrException.log(log, "Error closing old IndexWriter", t);
- }
+ writerPauseLock.wait();
+ } catch (InterruptedException e) {}
}
- indexWriter = createMainIndexWriter(core, "DirectUpdateHandler2", false,
- true);
- // we need to null this so it picks up the new writer next get call
- refCntWriter = null;
- } finally {
- pauseWriter = false;
- notifyAll();
+ try {
+ if (indexWriter != null) {
+ try {
+ indexWriter.close();
+ } catch (Throwable t) {
+ SolrException.log(log, "Error closing old IndexWriter", t);
+ }
+ }
+
+ indexWriter = createMainIndexWriter(core, "DirectUpdateHandler2",
+ false, true);
+ // we need to null this so it picks up the new writer next get call
+ refCntWriter = null;
+ } finally {
+
+ pauseWriter = false;
+ writerPauseLock.notifyAll();
+ }
}
}
@@ -161,7 +171,7 @@ public final class DefaultSolrCoreState
@Override
public synchronized void rollbackIndexWriter(SolrCore core) throws IOException {
indexWriter.rollback();
- newIndexWriter(core);
+ newIndexWriter(core, true);
}
protected SolrIndexWriter createMainIndexWriter(SolrCore core, String name,
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java?rev=1367171&r1=1367170&r2=1367171&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java Mon Jul 30 16:56:09 2012
@@ -343,7 +343,7 @@ public class DirectUpdateHandler2 extend
// 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) {
+ synchronized (solrCoreState.getUpdateLock()) {
deleteAll();
ulog.deleteAll();
return;
@@ -356,7 +356,7 @@ public class DirectUpdateHandler2 extend
// 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) {
+ synchronized (solrCoreState.getUpdateLock()) {
if (delAll) {
deleteAll();
} else {
@@ -392,7 +392,7 @@ public class DirectUpdateHandler2 extend
Term idTerm = new Term(idField.getName(), cmd.getIndexedId());
// see comment in deleteByQuery
- synchronized (solrCoreState) {
+ synchronized (solrCoreState.getUpdateLock()) {
RefCounted<IndexWriter> iw = solrCoreState.getIndexWriter(core);
try {
IndexWriter writer = iw.get();
@@ -518,7 +518,7 @@ public class DirectUpdateHandler2 extend
}
if (!cmd.softCommit) {
- synchronized (solrCoreState) { // sync is currently needed to prevent preCommit
+ synchronized (solrCoreState.getUpdateLock()) { // sync is currently needed to prevent preCommit
// from being called between preSoft and
// postSoft... see postSoft comments.
if (ulog != null) ulog.preCommit(cmd);
@@ -547,14 +547,14 @@ public class DirectUpdateHandler2 extend
if (cmd.softCommit) {
// ulog.preSoftCommit();
- synchronized (solrCoreState) {
+ synchronized (solrCoreState.getUpdateLock()) {
if (ulog != null) ulog.preSoftCommit(cmd);
core.getSearcher(true, false, waitSearcher, true);
if (ulog != null) ulog.postSoftCommit(cmd);
}
// ulog.postSoftCommit();
} else {
- synchronized (solrCoreState) {
+ synchronized (solrCoreState.getUpdateLock()) {
if (ulog != null) ulog.preSoftCommit(cmd);
if (cmd.openSearcher) {
core.getSearcher(true, false, waitSearcher);
@@ -606,8 +606,8 @@ public class DirectUpdateHandler2 extend
}
@Override
- public void newIndexWriter() throws IOException {
- solrCoreState.newIndexWriter(core);
+ public void newIndexWriter(boolean rollback) throws IOException {
+ solrCoreState.newIndexWriter(core, rollback);
}
/**
@@ -705,7 +705,7 @@ public class DirectUpdateHandler2 extend
// 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) {
+ synchronized (solrCoreState.getUpdateLock()) {
ulog.preCommit(cmd);
}
@@ -714,7 +714,7 @@ public class DirectUpdateHandler2 extend
commitData.put(SolrIndexWriter.COMMIT_TIME_MSEC_KEY, String.valueOf(System.currentTimeMillis()));
writer.commit(commitData);
- synchronized (solrCoreState) {
+ synchronized (solrCoreState.getUpdateLock()) {
ulog.postCommit(cmd);
}
}
@@ -823,4 +823,14 @@ public class DirectUpdateHandler2 extend
public void incref() {
solrCoreState.incref();
}
+
+ // allow access for tests
+ public CommitTracker getCommitTracker() {
+ return commitTracker;
+ }
+
+ // allow access for tests
+ public CommitTracker getSoftCommitTracker() {
+ return softCommitTracker;
+ }
}
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrCoreState.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrCoreState.java?rev=1367171&r1=1367170&r2=1367171&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrCoreState.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/SolrCoreState.java Mon Jul 30 16:56:09 2012
@@ -31,15 +31,21 @@ import org.apache.solr.util.RefCounted;
*
*/
public abstract class SolrCoreState {
+ private final Object deleteLock = new Object();
+
+ public Object getUpdateLock() {
+ return deleteLock;
+ }
/**
* Force the creation of a new IndexWriter using the settings from the given
* SolrCore.
*
* @param core
+ * @param rollback close IndexWriter if false, else rollback
* @throws IOException
*/
- public abstract void newIndexWriter(SolrCore core) throws IOException;
+ public abstract void newIndexWriter(SolrCore core, boolean rollback) throws IOException;
/**
* Get the current IndexWriter. If a new IndexWriter must be created, use the
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/UpdateHandler.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/UpdateHandler.java?rev=1367171&r1=1367170&r2=1367171&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/UpdateHandler.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/update/UpdateHandler.java Mon Jul 30 16:56:09 2012
@@ -127,9 +127,11 @@ public abstract class UpdateHandler impl
* Called when the Writer should be opened again - eg when replication replaces
* all of the index files.
*
+ * @param rollback IndexWriter if true else close
+ *
* @throws IOException
*/
- public abstract void newIndexWriter() throws IOException;
+ public abstract void newIndexWriter(boolean rollback) throws IOException;
public abstract SolrCoreState getSolrCoreState();
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/ChaosMonkeySafeLeaderTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/ChaosMonkeySafeLeaderTest.java?rev=1367171&r1=1367170&r2=1367171&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/ChaosMonkeySafeLeaderTest.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/ChaosMonkeySafeLeaderTest.java Mon Jul 30 16:56:09 2012
@@ -22,6 +22,9 @@ import java.util.List;
import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.servlet.SolrDispatchFilter;
+import org.apache.solr.update.DirectUpdateHandler2;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
@@ -35,7 +38,7 @@ public class ChaosMonkeySafeLeaderTest e
@BeforeClass
public static void beforeSuperClass() {
-
+
}
@AfterClass
@@ -77,9 +80,10 @@ public class ChaosMonkeySafeLeaderTest e
handle.put("QTime", SKIPVAL);
handle.put("timestamp", SKIPVAL);
- // we cannot do delete by query
- // as it's not supported for recovery
- //del("*:*");
+ // randomly turn on 5 seconds 'soft' commit
+ randomlyEnableAutoSoftCommit();
+
+ del("*:*");
List<StopableIndexingThread> threads = new ArrayList<StopableIndexingThread>();
int threadCount = 2;
@@ -116,6 +120,24 @@ public class ChaosMonkeySafeLeaderTest e
if (VERBOSE) System.out.println("control docs:" + controlClient.query(new SolrQuery("*:*")).getResults().getNumFound() + "\n\n");
}
+
+ private void randomlyEnableAutoSoftCommit() {
+ if (r.nextBoolean()) {
+ log.info("Turning on auto soft commit");
+ for (CloudJettyRunner jetty : shardToJetty.get("shard1")) {
+ SolrCore core = ((SolrDispatchFilter) jetty.jetty.getDispatchFilter()
+ .getFilter()).getCores().getCore("collection1");
+ try {
+ ((DirectUpdateHandler2) core.getUpdateHandler()).getCommitTracker()
+ .setTimeUpperBound(5000);
+ } finally {
+ core.close();
+ }
+ }
+ } else {
+ log.info("Not turning on auto soft commit");
+ }
+ }
// skip the randoms - they can deadlock...
protected void indexr(Object... fields) throws Exception {