Fix IndexReader ref leak when Lucene index modified and read in transaction

[timw]

What this PR does

This PR fixes an IndexReader ref leak in the ref counting logic of Lucene index searching that leads to:

• IndexReader ref counts leaking (i.e. not being decremented), which in turn leads to
• merges being blocked, leading to unlimited on-disk file growth (number of files and disk utilisation)
• index files tracked by the Lucene IndexFileDeleter remaining in memory, creating a memory leak (in the order of 100MB heap per million files)

The leak occurs when:

• a Lucene index is modified in a transaction, and subsequently
• a search is performed on that Lucene index

What we observed

Under a sustained workload (reading from a queue) that performed Lucene index based queries and also updated Lucene indexes in the same transaction, we observed:

• open files in the Lucene directory growing to large numbers (6000+) and then reducing, and continuing to cycle up and down
• undeleted index files in the Lucene index directory growing without bound, up to 2 million files
• the used heap in the OrientDB server growing continuously

When OrientDB was restarted, the first transaction to modify the Lucene index caused all of the unused index files to be deleted. Additionally, waiting 2 minutes for the IndexWriter to time out, and then performing a Lucene index query (causing the IndexWriter to reopen) has similar effects.

Heap dumps of the running system showed millions of files tracked in the IndexFileDeleter (using 100s of MB of heap), many with reference counts in the 100s of thousands, and many instances of IndexReader with ref counts in the thousands.

Analysis

When an IndexSearcher is constructed in a transaction containing pending Lucene index changes, a MultiReader is constructed across the FS based IndexReader (usually a SegmentReader and the in-memory IndexReader. If no pending changes have been made, then the underlying FS IndexReader is returned directly.

When the IndexSearcher wrapping this IndexReader is finished with in OLuceneResultSet (after exhaustion of the query iteration), the IndexReader underlying the searcher was being conditionally released.

Previously there was a guard on the release that did not release if the ref count was > 1. Aside from this being inherently suspect in a multi-threaded situation (it's checking an atomic integer), it essentially meant that the release was never called for the MultiReader case (since this reader always started life and got used with a ref count of 1).

MultiReader has 2 modes it can be used in:

• close readers, intended for use with bare IndexReaders where the MultiReader should take ownership of the underlying readers and close them when its own ref count hits 0
• not close readers, intended for use when a ``MultiReader is built across IndexReaders obtained from Lucene `ReferenceManagers` that are having their ref counts incremented/decremented externally to the `MultiReader`.

It looks like this refcount > 1 guard was a potentially mistaken attempt at avoiding MultiReader closing the underlying IndexReader, since it was being opened in close readers mode.
Unfortunately this meant that the FS IndexReader was never released when it was wrapped in a MultiReader, resulting in the file/memory leaks we observed.

Resolution

In this situation, we want the decrement of the IndexReader used in the search to always occur, regardless of whether it's a MultiReader or the base FS IndexReader.
To achieve this, we need to make sure the MultiReader has "ownership" of the IndexReaders we pass to it, and that it is switched into the reference counting/not closing mode. Passing ownership is achieved by decrementing the ref count of the wrapped IndexReaders after MultiReader has incremented them, negating the increment created on allocation, and ensuring MultiReader will decrement and potentially close the underlying readers appropriately.
With this change, the result set can simply release the IndexReader on completion of iteration, without needing to know what kind of reader it is.

We've tested this on 100s of thousands of transactions, and observed relatively small and constant numbers of index files throughout the same test scenarios, demonstrating that merging and deleting is being performed as expected.

We're still operating 3.1, but this patch applies cleanly to 3.2 as well.

[tglman]

Hi,

Thanks for this PR with the detail description, will double check it and merge it!