LUCENE-9478: Prevent DWPTDeleteQueue from referencing itself and leaking memory (#1779)

In LUCENE-9304 we introduced some fixes that unfortunately hold on to the previous
DWPTDeleteQueue which is essentially leaking IW memory and cause applications to fail.
This fixes the memory leak and adds a test to ensure its not leaking memory.
This commit is contained in:
Simon Willnauer 2020-08-24 21:29:27 +02:00 committed by GitHub
parent da095bc7da
commit 098f0dc8b4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 36 additions and 1 deletions

View File

@ -221,6 +221,14 @@ Other
--------------------- ---------------------
(No changes) (No changes)
======================= Lucene 8.6.2 =======================
Bug Fixes
---------------------
* LUCENE-9478: Prevent DWPTDeleteQueue from referencing itself and leaking memory. The queue
passed an implicit this reference to the next queue instance on flush which leaked about 500byte
of memory on each full flush, commit or getReader call. (Simon Willnauer)
======================= Lucene 8.6.1 ======================= ======================= Lucene 8.6.1 =======================
Bug Fixes Bug Fixes

View File

@ -573,6 +573,12 @@ final class DocumentsWriterDeleteQueue implements Accountable, Closeable {
} }
} }
// we use a static method to get this lambda since we previously introduced a memory leak since it would
// implicitly reference this.nextSeqNo which holds on to this del queue. see LUCENE-9478 for reference
private static LongSupplier getPrevMaxSeqIdSupplier(AtomicLong nextSeqNo) {
return () -> nextSeqNo.get() - 1;
}
/** /**
* Advances the queue to the next queue on flush. This carries over the the generation to the next queue and * Advances the queue to the next queue on flush. This carries over the the generation to the next queue and
* set the {@link #getMaxSeqNo()} based on the given maxNumPendingOps. This method can only be called once, subsequently * set the {@link #getMaxSeqNo()} based on the given maxNumPendingOps. This method can only be called once, subsequently
@ -593,7 +599,7 @@ final class DocumentsWriterDeleteQueue implements Accountable, Closeable {
return new DocumentsWriterDeleteQueue(infoStream, generation + 1, seqNo + 1, return new DocumentsWriterDeleteQueue(infoStream, generation + 1, seqNo + 1,
// don't pass ::getMaxCompletedSeqNo here b/c otherwise we keep an reference to this queue // don't pass ::getMaxCompletedSeqNo here b/c otherwise we keep an reference to this queue
// and this will be a memory leak since the queues can't be GCed // and this will be a memory leak since the queues can't be GCed
() -> nextSeqNo.get() - 1); getPrevMaxSeqIdSupplier(nextSeqNo));
} }

View File

@ -16,6 +16,7 @@
*/ */
package org.apache.lucene.index; package org.apache.lucene.index;
import java.lang.ref.WeakReference;
import java.util.HashSet; import java.util.HashSet;
import java.util.Set; import java.util.Set;
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CountDownLatch;
@ -36,6 +37,26 @@ import org.apache.lucene.util.ThreadInterruptedException;
*/ */
public class TestDocumentsWriterDeleteQueue extends LuceneTestCase { public class TestDocumentsWriterDeleteQueue extends LuceneTestCase {
public void testAdvanceReferencesOriginal() {
WeakAndNext weakAndNext = new WeakAndNext();
DocumentsWriterDeleteQueue next = weakAndNext.next;
assertNotNull(next);
System.gc();
assertNull(weakAndNext.weak.get());
}
class WeakAndNext {
final WeakReference<DocumentsWriterDeleteQueue> weak;
final DocumentsWriterDeleteQueue next;
WeakAndNext() {
DocumentsWriterDeleteQueue deleteQueue = new DocumentsWriterDeleteQueue(null);
weak = new WeakReference<>(deleteQueue);
next = deleteQueue.advanceQueue(2);
}
}
public void testUpdateDelteSlices() throws Exception { public void testUpdateDelteSlices() throws Exception {
DocumentsWriterDeleteQueue queue = new DocumentsWriterDeleteQueue(null); DocumentsWriterDeleteQueue queue = new DocumentsWriterDeleteQueue(null);
final int size = 200 + random().nextInt(500) * RANDOM_MULTIPLIER; final int size = 200 + random().nextInt(500) * RANDOM_MULTIPLIER;