Close unused readers immediately on refresh with the Lucene backend

Description

Currently, an explicit refresh with the Lucene backend will simply mark the current reader for closing on the next read.

I assume the motivation was to avoid closing readers too frequently, but I think this optimization is pointless:

  1. For explicit refreshes, this is an optimization we really don’t want to make, since one could be requesting refreshes precisely so that readers get closed and file descriptors get released, e.g. for a merge (see https://hibernate.atlassian.net/browse/HSEARCH-5107 ).

  2. For implicit refreshes, we only end up saving the cost of the synchronized block (since the reader’s closing will be done at some point anyway), and this can be achieved differently.

I think a valid implementation would be simply this:

@Override public void refresh() { if ( currentReaderEntry == null ) { return; } setCurrentReaderEntry( null ); }

And if we go that route, we’ll probably want to get rid of clear(), which will be virtually identical (just less optimized).

We’ll probably want to add a test to check that index files are no longer being “locked” after a refresh (e.g. they can be deleted on Windows) as long as no search query gets executed.

Activity

Show:
Fixed

Details

Assignee

Reporter

Components

Sprint

Fix versions

Priority

Created March 18, 2024 at 11:48 AM
Updated June 10, 2024 at 1:14 PM
Resolved May 20, 2024 at 9:26 AM