Lucene - Core
  1. Lucene - Core
  2. LUCENE-4398

"Memory Leak" in TermsHashPerField memory tracking

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.4
    • Fix Version/s: 3.6.2
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      I am witnessing an apparent leak in the memory tracking used to determine when a flush is necessary.

      Over time, this will result in every single document being flushed into its own segment as the memUsage will remain above the configured buffer size, causing a flush to be triggered after every add/update.

      Best I can figure, this is being caused by TermsHashPerField's tracking of memory usage for postingsHash and/or postingsArray combined with multi-threaded feeding.

      I suspect that the TermsHashPerField's postingsHash is growing in one thread, then, when a segment is flushed, a single, different thread will merge all TermsHashPerFields in FreqProxTermsWriter and then call shrinkHash(). I suspect this call of shrinkHash() is seeing an old postingsHash array, and subsequently not releasing all the memory that was allocated.

      If this is the case, I am also concerned that FreqProxTermsWriter will not write the correct terms into the index, although I have not confirmed that any indexing problem occurs as of yet.

      NOTE: i am witnessing this growth in a test by subtracting the amount or memory allocated (but in a "free" state) by perDocAllocator/byteBlockAllocator/charBlocks/intBlocks from DocumentsWriter.memUsage.get() in IndexWriter.doAfterFlush()
      I will see this stay at a stable point for a while, then on some flushes, i will see this grow by a couple of bytes, and all subsequent flushes will never go back down the the previous state

      I will continue to investigate and post any additional findings

      1. LUCENE-4398.patch
        8 kB
        Michael McCandless

        Activity

        Hide
        Tim Smith added a comment -

        More information:

        I started tracking the memory usage internally in TermsHashPerField
        this resulted in an internal AtomicLong that held the amount of memory that was "held" by this class

        i then added a finalize() method that dumped the memory held to stdout

        result:
        as soon as i witnessed the memory grow, i forced garbage collection (via yourkit profiler)
        i then saw the finalize methods were called and the memory held by all garbage collected TermsHashPerField instances equaled the amount of memory that was leaked

        looks like the DocumentsWriter is releasing thread states without freeing the bytesUsed()?

        NOTE: this puts my concerns about thread safety/improper indexing to rest

        Show
        Tim Smith added a comment - More information: I started tracking the memory usage internally in TermsHashPerField this resulted in an internal AtomicLong that held the amount of memory that was "held" by this class i then added a finalize() method that dumped the memory held to stdout result: as soon as i witnessed the memory grow, i forced garbage collection (via yourkit profiler) i then saw the finalize methods were called and the memory held by all garbage collected TermsHashPerField instances equaled the amount of memory that was leaked looks like the DocumentsWriter is releasing thread states without freeing the bytesUsed()? NOTE: this puts my concerns about thread safety/improper indexing to rest
        Hide
        Michael McCandless added a comment -

        Not good! Would be nice to boil this down to a small test case...

        Have you tested 3.6 to see if it also shows the leak?

        Show
        Michael McCandless added a comment - Not good! Would be nice to boil this down to a small test case... Have you tested 3.6 to see if it also shows the leak?
        Hide
        Tim Smith added a comment -

        Looks like the culprit is DocFieldProcessorPerThread.trimFields()

        this method "releases" fields that were not seen recently
        for each field, this leaks 16 bytes from DocumentsWriter.bytesUsed's memory accounting

        Show
        Tim Smith added a comment - Looks like the culprit is DocFieldProcessorPerThread.trimFields() this method "releases" fields that were not seen recently for each field, this leaks 16 bytes from DocumentsWriter.bytesUsed's memory accounting
        Hide
        Tim Smith added a comment -

        Found a easy "fix" for this:
        commenting out the "bytesUsed(postingsHashSize * RamUsageEstimator.NUM_BYTES_INT)" line from TermsHashPerField's constructor does the trick

        This results in not accounting for 16 bytes for each field for each thread, this being the same 16 bytes that were not being reclaimed by trimFields()

        I suppose a more robust means to fix this would be to add a "destroy()" method to the PerField interfaces that would release this memory (however that would be a rather large patch)

        Also found a relatively easy way to reproduce this:
        Feed N documents with fields A-M
        force flush
        Feed N documents with fields N-Z
        force flush
        Repeat

        it will take a long time to actually consume all the memory (more fields used in test should accelerate things)

        Show
        Tim Smith added a comment - Found a easy "fix" for this: commenting out the "bytesUsed(postingsHashSize * RamUsageEstimator.NUM_BYTES_INT)" line from TermsHashPerField's constructor does the trick This results in not accounting for 16 bytes for each field for each thread, this being the same 16 bytes that were not being reclaimed by trimFields() I suppose a more robust means to fix this would be to add a "destroy()" method to the PerField interfaces that would release this memory (however that would be a rather large patch) Also found a relatively easy way to reproduce this: Feed N documents with fields A-M force flush Feed N documents with fields N-Z force flush Repeat it will take a long time to actually consume all the memory (more fields used in test should accelerate things)
        Hide
        Tim Smith added a comment -

        NOTE: 16 bytes of "unaccounted" space in the postingsHash is actually much less than the object header fields require for TermsHashPerField

        so, i would argue that not accounting for this 16 bytes is a valid low-profile fix to this

        the only gotcha would be if trimFields() is ever called on a TermsHashPerField that has not been shrunk down to size due to a flush
        is this possible?
        even if possible, i expect this only occurs in the rare case of deep-down exceptions?
        in that case, if abort() is called, i suppose the abort() method can be updated to shrink down the hash as well (if this is safe to do)

        Show
        Tim Smith added a comment - NOTE: 16 bytes of "unaccounted" space in the postingsHash is actually much less than the object header fields require for TermsHashPerField so, i would argue that not accounting for this 16 bytes is a valid low-profile fix to this the only gotcha would be if trimFields() is ever called on a TermsHashPerField that has not been shrunk down to size due to a flush is this possible? even if possible, i expect this only occurs in the rare case of deep-down exceptions? in that case, if abort() is called, i suppose the abort() method can be updated to shrink down the hash as well (if this is safe to do)
        Hide
        Michael McCandless added a comment -

        Patch w/ test case and fix.

        The issue doesn't affect 4.x/5.x because on flush we completely clear the slate / allocate a new DWPT. I'll commit the test case to be sure...

        I added a InvertedDocConsumerPerField.close() method, and implemented it in TermsHashPerField to account for freeing up the RAM.

        Show
        Michael McCandless added a comment - Patch w/ test case and fix. The issue doesn't affect 4.x/5.x because on flush we completely clear the slate / allocate a new DWPT. I'll commit the test case to be sure... I added a InvertedDocConsumerPerField.close() method, and implemented it in TermsHashPerField to account for freeing up the RAM.
        Hide
        Michael McCandless added a comment -

        Thanks Tim!

        Show
        Michael McCandless added a comment - Thanks Tim!
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Michael McCandless
        http://svn.apache.org/viewvc?view=revision&revision=1386921

        LUCENE-4398: add test case

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Michael McCandless http://svn.apache.org/viewvc?view=revision&revision=1386921 LUCENE-4398 : add test case
        Hide
        Uwe Schindler added a comment -

        Closed after release.

        Show
        Uwe Schindler added a comment - Closed after release.

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Tim Smith
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development