You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by mi...@apache.org on 2015/02/26 20:36:44 UTC
svn commit: r1662544 - in /lucene/dev/branches/lucene6299/lucene/core/src:
java/org/apache/lucene/index/ test/org/apache/lucene/index/
Author: mikemccand
Date: Thu Feb 26 19:36:44 2015
New Revision: 1662544
URL: http://svn.apache.org/r1662544
Log:
LUCENE-6299: fix deleteAll to deduct from pendingNumDocs more carefully; fix addIndexes to do best-effort check up front and only reserve for real just before changing SIS
Modified:
lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/DocumentsWriter.java
lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java
lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
lucene/dev/branches/lucene6299/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMaxDocs.java
Modified: lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/DocumentsWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/DocumentsWriter.java?rev=1662544&r1=1662543&r2=1662544&view=diff
==============================================================================
--- lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/DocumentsWriter.java (original)
+++ lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/DocumentsWriter.java Thu Feb 26 19:36:44 2015
@@ -226,12 +226,14 @@ final class DocumentsWriter implements C
}
}
}
-
- synchronized void lockAndAbortAll(IndexWriter indexWriter) {
+
+ /** Returns how many documents were aborted. */
+ synchronized long lockAndAbortAll(IndexWriter indexWriter) {
assert indexWriter.holdsFullFlushLock();
if (infoStream.isEnabled("DW")) {
infoStream.message("DW", "lockAndAbortAll");
}
+ long abortedDocCount = 0;
boolean success = false;
try {
deleteQueue.clear();
@@ -239,12 +241,13 @@ final class DocumentsWriter implements C
for (int i = 0; i < limit; i++) {
final ThreadState perThread = perThreadPool.getThreadState(i);
perThread.lock();
- abortThreadState(perThread);
+ abortedDocCount += abortThreadState(perThread);
}
deleteQueue.clear();
flushControl.abortPendingFlushes();
flushControl.waitForFlush();
success = true;
+ return abortedDocCount;
} finally {
if (infoStream.isEnabled("DW")) {
infoStream.message("DW", "finished lockAndAbortAll success=" + success);
@@ -255,22 +258,28 @@ final class DocumentsWriter implements C
}
}
}
-
- private final void abortThreadState(final ThreadState perThread) {
+
+ /** Returns how many documents were aborted. */
+ private final int abortThreadState(final ThreadState perThread) {
assert perThread.isHeldByCurrentThread();
if (perThread.isActive()) { // we might be closed
if (perThread.isInitialized()) {
try {
- subtractFlushedNumDocs(perThread.dwpt.getNumDocsInRAM());
+ int abortedDocCount = perThread.dwpt.getNumDocsInRAM();
+ subtractFlushedNumDocs(abortedDocCount);
perThread.dwpt.abort();
+ return abortedDocCount;
} finally {
flushControl.doOnAbort(perThread);
}
} else {
flushControl.doOnAbort(perThread);
+ // This DWPT was never initialized so it has no indexed documents:
+ return 0;
}
} else {
assert closed;
+ return 0;
}
}
Modified: lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java?rev=1662544&r1=1662543&r2=1662544&view=diff
==============================================================================
--- lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java (original)
+++ lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/DocumentsWriterPerThread.java Thu Feb 26 19:36:44 2015
@@ -201,9 +201,9 @@ class DocumentsWriterPerThread {
/** Anything that will add N docs to the index should reserve first to
* make sure it's allowed. */
- private void reserveDoc() {
+ private void reserveOneDoc() {
if (pendingNumDocs.incrementAndGet() > IndexWriter.getActualMaxDocs()) {
- // Reserve failed
+ // Reserve failed: put the one doc back and throw exc:
pendingNumDocs.decrementAndGet();
throw new IllegalArgumentException("number of documents in the index cannot exceed " + IndexWriter.getActualMaxDocs());
}
@@ -212,7 +212,7 @@ class DocumentsWriterPerThread {
public void updateDocument(IndexDocument doc, Analyzer analyzer, Term delTerm) throws IOException, AbortingException {
testPoint("DocumentsWriterPerThread addDocument start");
assert deleteQueue != null;
- reserveDoc();
+ reserveOneDoc();
docState.doc = doc;
docState.analyzer = analyzer;
docState.docID = numDocsInRAM;
@@ -261,7 +261,7 @@ class DocumentsWriterPerThread {
// document, so the counter will be "wrong" in that case, but
// it's very hard to fix (we can't easily distinguish aborting
// vs non-aborting exceptions):
- reserveDoc();
+ reserveOneDoc();
docState.doc = doc;
docState.docID = numDocsInRAM;
docCount++;
Modified: lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java?rev=1662544&r1=1662543&r2=1662544&view=diff
==============================================================================
--- lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java (original)
+++ lucene/dev/branches/lucene6299/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java Thu Feb 26 19:36:44 2015
@@ -2056,13 +2056,16 @@ public class IndexWriter implements Clos
*/
try {
synchronized (fullFlushLock) {
- docWriter.lockAndAbortAll(this);
+ long abortedDocCount = docWriter.lockAndAbortAll(this);
+ pendingNumDocs.addAndGet(-abortedDocCount);
+
processEvents(false, true);
synchronized (this) {
try {
// Abort any running merges
abortMerges();
// Remove all segments
+ pendingNumDocs.addAndGet(-segmentInfos.totalDocCount());
segmentInfos.clear();
// Ask deleter to locate unreferenced files & remove them:
deleter.checkpoint(segmentInfos, false);
@@ -2078,7 +2081,7 @@ public class IndexWriter implements Clos
++changeCount;
segmentInfos.changed();
globalFieldNumberMap.clear();
- pendingNumDocs.set(0);
+
success = true;
} finally {
docWriter.unlockAllAfterAbortAll(this);
@@ -2351,11 +2354,10 @@ public class IndexWriter implements Clos
totalDocCount += sis.totalDocCount();
commits.add(sis);
}
-
- // Make sure adding the new documents to this index won't
- // exceed the limit:
- reserveDocs(totalDocCount);
+ // Best-effort up front check:
+ testReserveDocs(totalDocCount);
+
boolean success = false;
try {
for (SegmentInfos sis : commits) {
@@ -2390,6 +2392,10 @@ public class IndexWriter implements Clos
success = false;
try {
ensureOpen();
+
+ // Now reserve the docs, just before we update SIS:
+ reserveDocs(totalDocCount);
+
success = true;
} finally {
if (!success) {
@@ -2466,11 +2472,10 @@ public class IndexWriter implements Clos
for (CodecReader leaf : readers) {
numDocs += leaf.numDocs();
}
-
- // Make sure adding the new documents to this index won't
- // exceed the limit:
- reserveDocs(numDocs);
+ // Best-effort up front check:
+ testReserveDocs(numDocs);
+
final IOContext context = new IOContext(new MergeInfo(Math.toIntExact(numDocs), -1, false, -1));
// TODO: somehow we should fix this merge so it's
@@ -2563,6 +2568,10 @@ public class IndexWriter implements Clos
return;
}
ensureOpen();
+
+ // Now reserve the docs, just before we update SIS:
+ reserveDocs(numDocs);
+
segmentInfos.add(infoPerCommit);
checkpoint();
}
@@ -4627,14 +4636,30 @@ public class IndexWriter implements Clos
/** Anything that will add N docs to the index should reserve first to
* make sure it's allowed. This will throw {@code
* IllegalArgumentException} if it's not allowed. */
- private void reserveDocs(long numDocs) {
- if (pendingNumDocs.addAndGet(numDocs) > actualMaxDocs) {
- // Reserve failed
- pendingNumDocs.addAndGet(-numDocs);
- throw new IllegalArgumentException("number of documents in the index cannot exceed " + actualMaxDocs + " (current document count is " + pendingNumDocs.get() + "; added numDocs is " + numDocs + ")");
+ private void reserveDocs(long addedNumDocs) {
+ assert addedNumDocs >= 0;
+ if (pendingNumDocs.addAndGet(addedNumDocs) > actualMaxDocs) {
+ // Reserve failed: put the docs back and throw exc:
+ pendingNumDocs.addAndGet(-addedNumDocs);
+ tooManyDocs(addedNumDocs);
+ }
+ }
+
+ /** Does a best-effort check, that the current index would accept this many additional docs, but does not actually reserve them.
+ *
+ * @throws IllegalArgumentException if there would be too many docs */
+ private void testReserveDocs(long addedNumDocs) {
+ assert addedNumDocs >= 0;
+ if (pendingNumDocs.get() + addedNumDocs > actualMaxDocs) {
+ tooManyDocs(addedNumDocs);
}
}
+ private void tooManyDocs(long addedNumDocs) {
+ assert addedNumDocs >= 0;
+ throw new IllegalArgumentException("number of documents in the index cannot exceed " + actualMaxDocs + " (current document count is " + pendingNumDocs.get() + "; added numDocs is " + addedNumDocs + ")");
+ }
+
/** Wraps the incoming {@link Directory} so that we assign a per-thread
* {@link MergeRateLimiter} to all created {@link IndexOutput}s. */
private Directory addMergeRateLimiters(Directory in) {
Modified: lucene/dev/branches/lucene6299/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMaxDocs.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/lucene6299/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMaxDocs.java?rev=1662544&r1=1662543&r2=1662544&view=diff
==============================================================================
--- lucene/dev/branches/lucene6299/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMaxDocs.java (original)
+++ lucene/dev/branches/lucene6299/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterMaxDocs.java Thu Feb 26 19:36:44 2015
@@ -20,6 +20,7 @@ package org.apache.lucene.index;
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
+import java.util.concurrent.CountDownLatch;
import org.apache.lucene.document.Document;
import org.apache.lucene.document.Field;
@@ -36,7 +37,6 @@ import org.apache.lucene.util.LuceneTest
import org.apache.lucene.util.LuceneTestCase;
import org.apache.lucene.util.TestUtil;
import org.apache.lucene.util.TimeUnits;
-
import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite;
@SuppressCodecs({ "SimpleText", "Memory", "Direct" })
@@ -495,6 +495,160 @@ public class TestIndexWriterMaxDocs exte
}
w.deleteAll();
w.addDocument(new Document());
+ try {
+ w.addDocument(new Document());
+ fail("didn't hit exception");
+ } catch (IllegalArgumentException iae) {
+ // expected
+ }
+ w.close();
+ dir.close();
+ } finally {
+ restoreIndexWriterMaxDocs();
+ }
+ }
+
+ // LUCENE-6299
+ public void testDeleteAllAfterFlush() throws Exception {
+ setIndexWriterMaxDocs(2);
+ try {
+ Directory dir = newDirectory();
+ IndexWriter w = new IndexWriter(dir, new IndexWriterConfig(null));
+ w.addDocument(new Document());
+ w.getReader().close();
+ w.addDocument(new Document());
+ try {
+ w.addDocument(new Document());
+ fail("didn't hit exception");
+ } catch (IllegalArgumentException iae) {
+ // expected
+ }
+ w.deleteAll();
+ w.addDocument(new Document());
+ w.addDocument(new Document());
+ try {
+ w.addDocument(new Document());
+ fail("didn't hit exception");
+ } catch (IllegalArgumentException iae) {
+ // expected
+ }
+ w.close();
+ dir.close();
+ } finally {
+ restoreIndexWriterMaxDocs();
+ }
+ }
+
+ // LUCENE-6299
+ public void testDeleteAllAfterCommit() throws Exception {
+ setIndexWriterMaxDocs(2);
+ try {
+ Directory dir = newDirectory();
+ IndexWriter w = new IndexWriter(dir, new IndexWriterConfig(null));
+ w.addDocument(new Document());
+ w.commit();
+ w.addDocument(new Document());
+ try {
+ w.addDocument(new Document());
+ fail("didn't hit exception");
+ } catch (IllegalArgumentException iae) {
+ // expected
+ }
+ w.deleteAll();
+ w.addDocument(new Document());
+ w.addDocument(new Document());
+ try {
+ w.addDocument(new Document());
+ fail("didn't hit exception");
+ } catch (IllegalArgumentException iae) {
+ // expected
+ }
+ w.close();
+ dir.close();
+ } finally {
+ restoreIndexWriterMaxDocs();
+ }
+ }
+
+ // LUCENE-6299
+ public void testDeleteAllMultipleThreads() throws Exception {
+ int limit = TestUtil.nextInt(random(), 2, 10);
+ setIndexWriterMaxDocs(limit);
+ try {
+ Directory dir = newDirectory();
+ IndexWriter w = new IndexWriter(dir, new IndexWriterConfig(null));
+
+ CountDownLatch startingGun = new CountDownLatch(1);
+ Thread[] threads = new Thread[limit];
+ for(int i=0;i<limit;i++) {
+ threads[i] = new Thread() {
+ @Override
+ public void run() {
+ try {
+ startingGun.await();
+ w.addDocument(new Document());
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+ };
+ threads[i].start();
+ }
+
+ startingGun.countDown();
+
+ for(Thread thread : threads) {
+ thread.join();
+ }
+
+ try {
+ w.addDocument(new Document());
+ fail("didn't hit exception");
+ } catch (IllegalArgumentException iae) {
+ // expected
+ }
+ w.deleteAll();
+ for(int i=0;i<limit;i++) {
+ w.addDocument(new Document());
+ }
+ try {
+ w.addDocument(new Document());
+ fail("didn't hit exception");
+ } catch (IllegalArgumentException iae) {
+ // expected
+ }
+ w.close();
+ dir.close();
+ } finally {
+ restoreIndexWriterMaxDocs();
+ }
+ }
+
+ // LUCENE-6299
+ public void testDeleteAllAfterClose() throws Exception {
+ setIndexWriterMaxDocs(2);
+ try {
+ Directory dir = newDirectory();
+ IndexWriter w = new IndexWriter(dir, new IndexWriterConfig(null));
+ w.addDocument(new Document());
+ w.close();
+ w = new IndexWriter(dir, new IndexWriterConfig(null));
+ w.addDocument(new Document());
+ try {
+ w.addDocument(new Document());
+ fail("didn't hit exception");
+ } catch (IllegalArgumentException iae) {
+ // expected
+ }
+ w.deleteAll();
+ w.addDocument(new Document());
+ w.addDocument(new Document());
+ try {
+ w.addDocument(new Document());
+ fail("didn't hit exception");
+ } catch (IllegalArgumentException iae) {
+ // expected
+ }
w.close();
dir.close();
} finally {