Details
-
Improvement
-
Status: Closed
-
Minor
-
Resolution: Implemented
-
None
-
None
-
New, Patch Available
Description
DirTaxoReader uses hairy code to protect its indexReader instance from
being modified while threads use it. It maintains a ReentrantLock
(indexReaderLock) which is obtained on every 'read' access, while
refresh() locks it for 'write' operations (refreshing the IndexReader).
Instead of all that, now that we have ReferenceManager in place, I think
that we can write a ReaderManager<IndexReader> which will be used by
DirTR. Every method that requires access to the indexReader will
acquire/release (not too different than obtaining/releasing the read
lock), and refresh() will call ReaderManager.maybeRefresh(). It will
simplify the code and remove some rather long comments, that go into
great length explaining why does the code looks like that.
This ReaderManager cannot be used for every IndexReader, because DirTR's
refresh() logic is special – it reopens the indexReader, and then
verifies that the createTime still matches on the reopened reader as
well. Otherwise, it closes the reopened reader and fails with an exception.
Therefore, this ReaderManager.refreshIfNeeded will need to take the
createTime into consideration and fail if they do not match.
And while we're at it ... I wonder if we should have a manager for an
IndexReader/ParentArray pair? I think that it makes sense because we
don't want DirTR to use a ParentArray that does not match the IndexReader.
Today this can happen in refresh() if e.g. after the indexReader instance
has been replaced, parentArray.refresh(indexReader) fails. DirTR will be
left with a newer IndexReader instance, but old (or worse, corrupt?)
ParentArray ... I think it'll be good if we introduce clone() on ParentArray,
or a new ctor which takes an int[].
I'll work on a patch once I finish with LUCENE-3786.
Attachments
Issue Links
- Is contained by
-
LUCENE-3441 Add NRT support to facets
-
- Closed
-