From 66ecf9afb52fab64cbd3d5e55e4cf402cb02995e Mon Sep 17 00:00:00 2001 From: Mark Robert Miller Date: Fri, 8 Feb 2013 17:20:05 +0000 Subject: [PATCH] SOLR-4400: Deadlock can occur in a rare race between committing and closing a SolrIndexWriter. git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1444152 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 3 ++ .../solr/update/DefaultSolrCoreState.java | 34 +++++++++++++++---- .../solr/update/DirectUpdateHandler2.java | 11 +++--- .../org/apache/solr/update/SolrCoreState.java | 3 ++ 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 3075e4f0e62..46c397897d6 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -108,6 +108,9 @@ Bug Fixes * SOLR-4380: Replicate after startup option would not replicate until the IndexWriter was lazily opened. (Mark Miller, Gregg Donovan) +* SOLR-4400: Deadlock can occur in a rare race between committing and + closing a SolrIndexWriter. (Erick Erickson, Mark Miller) + Optimizations ---------------------- diff --git a/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java b/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java index e98ce048990..031bd27b350 100644 --- a/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java +++ b/solr/core/src/java/org/apache/solr/update/DefaultSolrCoreState.java @@ -18,6 +18,8 @@ package org.apache.solr.update; */ import java.io.IOException; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.apache.lucene.index.IndexWriter; import org.apache.lucene.store.AlreadyClosedException; @@ -52,6 +54,8 @@ public final class DefaultSolrCoreState extends SolrCoreState implements Recover private boolean pauseWriter; private boolean writerFree = true; + protected final ReentrantLock commitLock = new ReentrantLock(); + public DefaultSolrCoreState(DirectoryFactory directoryFactory) { this.directoryFactory = directoryFactory; } @@ -135,13 +139,26 @@ public final class DefaultSolrCoreState extends SolrCoreState implements Recover pauseWriter = true; // then lets wait until its out of use log.info("Waiting until IndexWriter is unused... core=" + coreName); - while (!writerFree) { - try { - writerPauseLock.wait(100); - } catch (InterruptedException e) {} + + boolean yieldedCommitLock = false; + try { + if (commitLock.isHeldByCurrentThread()) { + yieldedCommitLock = true; + commitLock.unlock(); + } - if (closed) { - throw new RuntimeException("SolrCoreState already closed"); + while (!writerFree) { + try { + writerPauseLock.wait(100); + } catch (InterruptedException e) {} + + if (closed) { + throw new RuntimeException("SolrCoreState already closed"); + } + } + } finally { + if (yieldedCommitLock) { + commitLock.lock(); } } @@ -272,4 +289,9 @@ public final class DefaultSolrCoreState extends SolrCoreState implements Recover closeIndexWriter(closer); } + @Override + public Lock getCommitLock() { + return commitLock; + } + } diff --git a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java index eea3df74970..8a82f79dfbf 100644 --- a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java +++ b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java @@ -29,8 +29,6 @@ import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; import org.apache.lucene.document.Document; import org.apache.lucene.index.IndexReader; @@ -70,7 +68,6 @@ import org.apache.solr.util.RefCounted; */ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState.IndexWriterCloser { protected final SolrCoreState solrCoreState; - protected final Lock commitLock = new ReentrantLock(); // stats AtomicLong addCommands = new AtomicLong(); @@ -502,7 +499,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState try { // only allow one hard commit to proceed at once if (!cmd.softCommit) { - commitLock.lock(); + solrCoreState.getCommitLock().lock(); } log.info("start "+cmd); @@ -596,7 +593,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState } finally { if (!cmd.softCommit) { - commitLock.unlock(); + solrCoreState.getCommitLock().unlock(); } addCommands.set(0); @@ -680,7 +677,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState @Override public void closeWriter(IndexWriter writer) throws IOException { boolean clearRequestInfo = false; - commitLock.lock(); + solrCoreState.getCommitLock().lock(); try { SolrQueryRequest req = new LocalSolrQueryRequest(core, new ModifiableSolrParams()); SolrQueryResponse rsp = new SolrQueryResponse(); @@ -745,7 +742,7 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState if (writer != null) writer.close(); } finally { - commitLock.unlock(); + solrCoreState.getCommitLock().unlock(); if (clearRequestInfo) SolrRequestInfo.clearRequestInfo(); } } diff --git a/solr/core/src/java/org/apache/solr/update/SolrCoreState.java b/solr/core/src/java/org/apache/solr/update/SolrCoreState.java index d9add2d7ea9..738100163fb 100644 --- a/solr/core/src/java/org/apache/solr/update/SolrCoreState.java +++ b/solr/core/src/java/org/apache/solr/update/SolrCoreState.java @@ -18,6 +18,7 @@ package org.apache.solr.update; */ import java.io.IOException; +import java.util.concurrent.locks.Lock; import org.apache.lucene.index.IndexWriter; import org.apache.solr.core.CoreContainer; @@ -37,6 +38,8 @@ public abstract class SolrCoreState { return deleteLock; } + public abstract Lock getCommitLock(); + /** * Force the creation of a new IndexWriter using the settings from the given * SolrCore.