You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by jp...@apache.org on 2017/01/25 15:40:39 UTC

[1/2] lucene-solr:master: LUCENE-7657: Fixed potential memory leak when a (Span)TermQuery that wraps a TermContext is cached.

Repository: lucene-solr
Updated Branches:
  refs/heads/master 9899cbd03 -> 94530940e


LUCENE-7657: Fixed potential memory leak when a (Span)TermQuery that wraps a TermContext is cached.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/f5301428
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/f5301428
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/f5301428

Branch: refs/heads/master
Commit: f5301428452ee5f9145ef4ecb889442d4e09f1cb
Parents: 9899cbd
Author: Adrien Grand <jp...@gmail.com>
Authored: Wed Jan 25 16:10:47 2017 +0100
Committer: Adrien Grand <jp...@gmail.com>
Committed: Wed Jan 25 16:11:47 2017 +0100

----------------------------------------------------------------------
 lucene/CHANGES.txt                              |  7 +++++++
 .../apache/lucene/index/IndexReaderContext.java |  7 ++++++-
 .../org/apache/lucene/index/TermContext.java    | 21 ++++++++++++--------
 .../apache/lucene/search/BlendedTermQuery.java  | 12 ++++++-----
 .../org/apache/lucene/search/TermQuery.java     |  6 +++---
 .../lucene/search/spans/SpanTermQuery.java      |  4 ++--
 .../lucene/search/TermAutomatonQuery.java       |  2 +-
 7 files changed, 39 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f5301428/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index deb7078..a68d7e3 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -118,6 +118,13 @@ Build
 
 * LUCENE-7653: Update randomizedtesting to version 2.5.0. (Dawid Weiss)
 
+======================= Lucene 6.4.1 =======================
+
+Bug Fixes
+
+* LUCENE-7657: Fixed potential memory leak in the case that a (Span)TermQuery
+  with a TermContext is cached. (Adrien Grand)
+
 ======================= Lucene 6.4.0 =======================
 
 API Changes

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f5301428/lucene/core/src/java/org/apache/lucene/index/IndexReaderContext.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/IndexReaderContext.java b/lucene/core/src/java/org/apache/lucene/index/IndexReaderContext.java
index 247fa57..dada3ff 100644
--- a/lucene/core/src/java/org/apache/lucene/index/IndexReaderContext.java
+++ b/lucene/core/src/java/org/apache/lucene/index/IndexReaderContext.java
@@ -32,7 +32,12 @@ public abstract class IndexReaderContext {
   public final int docBaseInParent;
   /** the ord for this reader in the parent, <tt>0</tt> if parent is null */
   public final int ordInParent;
-  
+
+  // An object that uniquely identifies this context without referencing
+  // segments. The goal is to make it fine to have references to this
+  // identity object, even after the index reader has been closed
+  final Object identity = new Object();
+
   IndexReaderContext(CompositeReaderContext parent, int ordInParent, int docBaseInParent) {
     if (!(this instanceof CompositeReaderContext || this instanceof LeafReaderContext))
       throw new Error("This class should never be extended by custom code!");

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f5301428/lucene/core/src/java/org/apache/lucene/index/TermContext.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/index/TermContext.java b/lucene/core/src/java/org/apache/lucene/index/TermContext.java
index e55aeba..ed25564 100644
--- a/lucene/core/src/java/org/apache/lucene/index/TermContext.java
+++ b/lucene/core/src/java/org/apache/lucene/index/TermContext.java
@@ -33,12 +33,8 @@ import java.util.Arrays;
  */
 public final class TermContext {
 
-  /** Holds the {@link IndexReaderContext} of the top-level
-   *  {@link IndexReader}, used internally only for
-   *  asserting.
-   *
-   *  @lucene.internal */
-  public final IndexReaderContext topReaderContext;
+  // Important: do NOT keep hard references to index readers
+  private final Object topReaderContextIdentity;
   private final TermState[] states;
   private int docFreq;
   private long totalTermFreq;
@@ -50,7 +46,7 @@ public final class TermContext {
    */
   public TermContext(IndexReaderContext context) {
     assert context != null && context.isTopLevel;
-    topReaderContext = context;
+    topReaderContextIdentity = context.identity;
     docFreq = 0;
     totalTermFreq = 0;
     final int len;
@@ -61,7 +57,16 @@ public final class TermContext {
     }
     states = new TermState[len];
   }
-  
+
+  /**
+   * Expert: Return whether this {@link TermContext} was built for the given
+   * {@link IndexReaderContext}. This is typically used for assertions.
+   * @lucene.internal
+   */
+  public boolean wasBuiltFor(IndexReaderContext context) {
+    return topReaderContextIdentity == context.identity;
+  }
+
   /**
    * Creates a {@link TermContext} with an initial {@link TermState},
    * {@link IndexReader} pair.

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f5301428/lucene/core/src/java/org/apache/lucene/search/BlendedTermQuery.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/search/BlendedTermQuery.java b/lucene/core/src/java/org/apache/lucene/search/BlendedTermQuery.java
index 85b8b0a..3a0cdc5 100644
--- a/lucene/core/src/java/org/apache/lucene/search/BlendedTermQuery.java
+++ b/lucene/core/src/java/org/apache/lucene/search/BlendedTermQuery.java
@@ -22,6 +22,7 @@ import java.util.Arrays;
 import java.util.List;
 
 import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.IndexReaderContext;
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.index.TermContext;
@@ -264,7 +265,7 @@ public final class BlendedTermQuery extends Query {
   public final Query rewrite(IndexReader reader) throws IOException {
     final TermContext[] contexts = Arrays.copyOf(this.contexts, this.contexts.length);
     for (int i = 0; i < contexts.length; ++i) {
-      if (contexts[i] == null || contexts[i].topReaderContext != reader.getContext()) {
+      if (contexts[i] == null || contexts[i].wasBuiltFor(reader.getContext()) == false) {
         contexts[i] = TermContext.build(reader.getContext(), terms[i]);
       }
     }
@@ -284,7 +285,7 @@ public final class BlendedTermQuery extends Query {
     }
 
     for (int i = 0; i < contexts.length; ++i) {
-      contexts[i] = adjustFrequencies(contexts[i], df, ttf);
+      contexts[i] = adjustFrequencies(reader.getContext(), contexts[i], df, ttf);
     }
 
     Query[] termQueries = new Query[terms.length];
@@ -297,15 +298,16 @@ public final class BlendedTermQuery extends Query {
     return rewriteMethod.rewrite(termQueries);
   }
 
-  private static TermContext adjustFrequencies(TermContext ctx, int artificialDf, long artificialTtf) {
-    List<LeafReaderContext> leaves = ctx.topReaderContext.leaves();
+  private static TermContext adjustFrequencies(IndexReaderContext readerContext,
+      TermContext ctx, int artificialDf, long artificialTtf) {
+    List<LeafReaderContext> leaves = readerContext.leaves();
     final int len;
     if (leaves == null) {
       len = 1;
     } else {
       len = leaves.size();
     }
-    TermContext newCtx = new TermContext(ctx.topReaderContext);
+    TermContext newCtx = new TermContext(readerContext);
     for (int i = 0; i < len; ++i) {
       TermState termState = ctx.get(i);
       if (termState == null) {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f5301428/lucene/core/src/java/org/apache/lucene/search/TermQuery.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/search/TermQuery.java b/lucene/core/src/java/org/apache/lucene/search/TermQuery.java
index 73170b9..e3e299f 100644
--- a/lucene/core/src/java/org/apache/lucene/search/TermQuery.java
+++ b/lucene/core/src/java/org/apache/lucene/search/TermQuery.java
@@ -86,7 +86,7 @@ public class TermQuery extends Query {
 
     @Override
     public Scorer scorer(LeafReaderContext context) throws IOException {
-      assert termStates == null || termStates.topReaderContext == ReaderUtil.getTopLevelContext(context) : "The top-reader used to create Weight (" + termStates.topReaderContext + ") is not the same as the current reader's top-reader (" + ReaderUtil.getTopLevelContext(context);;
+      assert termStates == null || termStates.wasBuiltFor(ReaderUtil.getTopLevelContext(context)) : "The top-reader used to create Weight is not the same as the current reader's top-reader (" + ReaderUtil.getTopLevelContext(context);;
       final TermsEnum termsEnum = getTermsEnum(context);
       if (termsEnum == null) {
         return null;
@@ -103,7 +103,7 @@ public class TermQuery extends Query {
     private TermsEnum getTermsEnum(LeafReaderContext context) throws IOException {
       if (termStates != null) {
         // TermQuery either used as a Query or the term states have been provided at construction time
-        assert termStates.topReaderContext == ReaderUtil.getTopLevelContext(context) : "The top-reader used to create Weight (" + termStates.topReaderContext + ") is not the same as the current reader's top-reader (" + ReaderUtil.getTopLevelContext(context);
+        assert termStates.wasBuiltFor(ReaderUtil.getTopLevelContext(context)) : "The top-reader used to create Weight is not the same as the current reader's top-reader (" + ReaderUtil.getTopLevelContext(context);
         final TermState state = termStates.get(context.ord);
         if (state == null) { // term is not present in that reader
           assert termNotInReader(context.reader(), term) : "no termstate found but term exists in reader term=" + term;
@@ -181,7 +181,7 @@ public class TermQuery extends Query {
     final IndexReaderContext context = searcher.getTopReaderContext();
     final TermContext termState;
     if (perReaderTermState == null
-        || perReaderTermState.topReaderContext != context) {
+        || perReaderTermState.wasBuiltFor(context) == false) {
       if (needsScores) {
         // make TermQuery single-pass if we don't have a PRTS or if the context
         // differs!

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f5301428/lucene/core/src/java/org/apache/lucene/search/spans/SpanTermQuery.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/search/spans/SpanTermQuery.java b/lucene/core/src/java/org/apache/lucene/search/spans/SpanTermQuery.java
index 2746a0c..3e13be7 100644
--- a/lucene/core/src/java/org/apache/lucene/search/spans/SpanTermQuery.java
+++ b/lucene/core/src/java/org/apache/lucene/search/spans/SpanTermQuery.java
@@ -67,7 +67,7 @@ public class SpanTermQuery extends SpanQuery {
   public SpanWeight createWeight(IndexSearcher searcher, boolean needsScores, float boost) throws IOException {
     final TermContext context;
     final IndexReaderContext topContext = searcher.getTopReaderContext();
-    if (termContext == null || termContext.topReaderContext != topContext) {
+    if (termContext == null || termContext.wasBuiltFor(topContext) == false) {
       context = TermContext.build(topContext, term);
     }
     else {
@@ -99,7 +99,7 @@ public class SpanTermQuery extends SpanQuery {
     @Override
     public Spans getSpans(final LeafReaderContext context, Postings requiredPostings) throws IOException {
 
-      assert termContext.topReaderContext == ReaderUtil.getTopLevelContext(context) : "The top-reader used to create Weight (" + termContext.topReaderContext + ") is not the same as the current reader's top-reader (" + ReaderUtil.getTopLevelContext(context);
+      assert termContext.wasBuiltFor(ReaderUtil.getTopLevelContext(context)) : "The top-reader used to create Weight is not the same as the current reader's top-reader (" + ReaderUtil.getTopLevelContext(context);
 
       final TermState state = termContext.get(context.ord);
       if (state == null) { // term is not present in that reader

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f5301428/lucene/sandbox/src/java/org/apache/lucene/search/TermAutomatonQuery.java
----------------------------------------------------------------------
diff --git a/lucene/sandbox/src/java/org/apache/lucene/search/TermAutomatonQuery.java b/lucene/sandbox/src/java/org/apache/lucene/search/TermAutomatonQuery.java
index fbf3dc3..04c8736 100644
--- a/lucene/sandbox/src/java/org/apache/lucene/search/TermAutomatonQuery.java
+++ b/lucene/sandbox/src/java/org/apache/lucene/search/TermAutomatonQuery.java
@@ -378,7 +378,7 @@ public class TermAutomatonQuery extends Query {
       boolean any = false;
       for(Map.Entry<Integer,TermContext> ent : termStates.entrySet()) {
         TermContext termContext = ent.getValue();
-        assert termContext.topReaderContext == ReaderUtil.getTopLevelContext(context) : "The top-reader used to create Weight (" + termContext.topReaderContext + ") is not the same as the current reader's top-reader (" + ReaderUtil.getTopLevelContext(context);
+        assert termContext.wasBuiltFor(ReaderUtil.getTopLevelContext(context)) : "The top-reader used to create Weight is not the same as the current reader's top-reader (" + ReaderUtil.getTopLevelContext(context);
         BytesRef term = idToTerm.get(ent.getKey());
         TermState state = termContext.get(context.ord);
         if (state != null) {


[2/2] lucene-solr:master: LUCENE-7647: CompressingStoredFieldsFormat should reclaim memory more aggressively.

Posted by jp...@apache.org.
LUCENE-7647: CompressingStoredFieldsFormat should reclaim memory more aggressively.


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/94530940
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/94530940
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/94530940

Branch: refs/heads/master
Commit: 94530940e4de8b476a5886f284578c933a8f33ef
Parents: f530142
Author: Adrien Grand <jp...@gmail.com>
Authored: Wed Jan 25 16:15:04 2017 +0100
Committer: Adrien Grand <jp...@gmail.com>
Committed: Wed Jan 25 16:15:04 2017 +0100

----------------------------------------------------------------------
 lucene/CHANGES.txt                              |  4 ++
 .../CompressingStoredFieldsWriter.java          |  5 +-
 .../codecs/compressing/CompressionMode.java     | 49 ++++++++++++++------
 .../lucene/codecs/compressing/Compressor.java   |  3 +-
 .../dummy/DummyCompressingCodec.java            |  3 ++
 5 files changed, 46 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/94530940/lucene/CHANGES.txt
----------------------------------------------------------------------
diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index a68d7e3..9a1622f 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -125,6 +125,10 @@ Bug Fixes
 * LUCENE-7657: Fixed potential memory leak in the case that a (Span)TermQuery
   with a TermContext is cached. (Adrien Grand)
 
+* LUCENE-7647: Made stored fields reclaim native memory more aggressively when
+  configured with BEST_COMPRESSION. This could otherwise result in out-of-memory
+  issues. (Adrien Grand)
+
 ======================= Lucene 6.4.0 =======================
 
 API Changes

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/94530940/lucene/core/src/java/org/apache/lucene/codecs/compressing/CompressingStoredFieldsWriter.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/codecs/compressing/CompressingStoredFieldsWriter.java b/lucene/core/src/java/org/apache/lucene/codecs/compressing/CompressingStoredFieldsWriter.java
index 7ab20af..5b42870 100644
--- a/lucene/core/src/java/org/apache/lucene/codecs/compressing/CompressingStoredFieldsWriter.java
+++ b/lucene/core/src/java/org/apache/lucene/codecs/compressing/CompressingStoredFieldsWriter.java
@@ -81,7 +81,7 @@ public final class CompressingStoredFieldsWriter extends StoredFieldsWriter {
   private CompressingStoredFieldsIndexWriter indexWriter;
   private IndexOutput fieldsStream;
 
-  private final Compressor compressor;
+  private Compressor compressor;
   private final CompressionMode compressionMode;
   private final int chunkSize;
   private final int maxDocsPerChunk;
@@ -141,10 +141,11 @@ public final class CompressingStoredFieldsWriter extends StoredFieldsWriter {
   @Override
   public void close() throws IOException {
     try {
-      IOUtils.close(fieldsStream, indexWriter);
+      IOUtils.close(fieldsStream, indexWriter, compressor);
     } finally {
       fieldsStream = null;
       indexWriter = null;
+      compressor = null;
     }
   }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/94530940/lucene/core/src/java/org/apache/lucene/codecs/compressing/CompressionMode.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/codecs/compressing/CompressionMode.java b/lucene/core/src/java/org/apache/lucene/codecs/compressing/CompressionMode.java
index 326eba3..53a84cb 100644
--- a/lucene/core/src/java/org/apache/lucene/codecs/compressing/CompressionMode.java
+++ b/lucene/core/src/java/org/apache/lucene/codecs/compressing/CompressionMode.java
@@ -164,6 +164,10 @@ public abstract class CompressionMode {
       LZ4.compress(bytes, off, len, out, ht);
     }
 
+    @Override
+    public void close() throws IOException {
+      // no-op
+    }
   }
 
   private static final class LZ4HighCompressor extends Compressor {
@@ -180,15 +184,17 @@ public abstract class CompressionMode {
       LZ4.compressHC(bytes, off, len, out, ht);
     }
 
+    @Override
+    public void close() throws IOException {
+      // no-op
+    }
   }
 
   private static final class DeflateDecompressor extends Decompressor {
 
-    final Inflater decompressor;
     byte[] compressed;
 
     DeflateDecompressor() {
-      decompressor = new Inflater(true);
       compressed = new byte[0];
     }
 
@@ -207,20 +213,24 @@ public abstract class CompressionMode {
       in.readBytes(compressed, 0, compressedLength);
       compressed[compressedLength] = 0; // explicitly set dummy byte to 0
 
-      decompressor.reset();
-      // extra "dummy byte"
-      decompressor.setInput(compressed, 0, paddedLength);
-
-      bytes.offset = bytes.length = 0;
-      bytes.bytes = ArrayUtil.grow(bytes.bytes, originalLength);
+      final Inflater decompressor = new Inflater(true);
       try {
-        bytes.length = decompressor.inflate(bytes.bytes, bytes.length, originalLength);
-      } catch (DataFormatException e) {
-        throw new IOException(e);
-      }
-      if (!decompressor.finished()) {
-        throw new CorruptIndexException("Invalid decoder state: needsInput=" + decompressor.needsInput() 
-                                                            + ", needsDict=" + decompressor.needsDictionary(), in);
+        // extra "dummy byte"
+        decompressor.setInput(compressed, 0, paddedLength);
+
+        bytes.offset = bytes.length = 0;
+        bytes.bytes = ArrayUtil.grow(bytes.bytes, originalLength);
+        try {
+          bytes.length = decompressor.inflate(bytes.bytes, bytes.length, originalLength);
+        } catch (DataFormatException e) {
+          throw new IOException(e);
+        }
+        if (!decompressor.finished()) {
+          throw new CorruptIndexException("Invalid decoder state: needsInput=" + decompressor.needsInput()
+                                                              + ", needsDict=" + decompressor.needsDictionary(), in);
+        }
+      } finally {
+        decompressor.end();
       }
       if (bytes.length != originalLength) {
         throw new CorruptIndexException("Lengths mismatch: " + bytes.length + " != " + originalLength, in);
@@ -240,6 +250,7 @@ public abstract class CompressionMode {
 
     final Deflater compressor;
     byte[] compressed;
+    boolean closed;
 
     DeflateCompressor(int level) {
       compressor = new Deflater(level, true);
@@ -275,6 +286,14 @@ public abstract class CompressionMode {
       out.writeBytes(compressed, totalCount);
     }
 
+    @Override
+    public void close() throws IOException {
+      if (closed == false) {
+        compressor.end();
+        closed = true;
+      }
+    }
+
   }
 
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/94530940/lucene/core/src/java/org/apache/lucene/codecs/compressing/Compressor.java
----------------------------------------------------------------------
diff --git a/lucene/core/src/java/org/apache/lucene/codecs/compressing/Compressor.java b/lucene/core/src/java/org/apache/lucene/codecs/compressing/Compressor.java
index bd2fadb..f95246c 100644
--- a/lucene/core/src/java/org/apache/lucene/codecs/compressing/Compressor.java
+++ b/lucene/core/src/java/org/apache/lucene/codecs/compressing/Compressor.java
@@ -17,6 +17,7 @@
 package org.apache.lucene.codecs.compressing;
 
 
+import java.io.Closeable;
 import java.io.IOException;
 
 import org.apache.lucene.store.DataOutput;
@@ -24,7 +25,7 @@ import org.apache.lucene.store.DataOutput;
 /**
  * A data compressor.
  */
-public abstract class Compressor {
+public abstract class Compressor implements Closeable {
 
   /** Sole constructor, typically called from sub-classes. */
   protected Compressor() {}

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/94530940/lucene/test-framework/src/java/org/apache/lucene/codecs/compressing/dummy/DummyCompressingCodec.java
----------------------------------------------------------------------
diff --git a/lucene/test-framework/src/java/org/apache/lucene/codecs/compressing/dummy/DummyCompressingCodec.java b/lucene/test-framework/src/java/org/apache/lucene/codecs/compressing/dummy/DummyCompressingCodec.java
index d15adad..167418e 100644
--- a/lucene/test-framework/src/java/org/apache/lucene/codecs/compressing/dummy/DummyCompressingCodec.java
+++ b/lucene/test-framework/src/java/org/apache/lucene/codecs/compressing/dummy/DummyCompressingCodec.java
@@ -79,6 +79,9 @@ public class DummyCompressingCodec extends CompressingCodec {
       out.writeBytes(bytes, off, len);
     }
 
+    @Override
+    public void close() throws IOException {};
+
   };
 
   /** Constructor that allows to configure the chunk size. */