Uploaded image for project: 'Lucene - Core'
  1. Lucene - Core
  2. LUCENE-3793

Use ReferenceManager in DirectoryTaxonomyReader


    • Improvement
    • Status: Closed
    • Minor
    • Resolution: Implemented
    • None
    • 4.1
    • modules/facet
    • None
    • New, Patch Available


      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.


        Issue Links



              shaie Shai Erera
              shaie Shai Erera
              0 Vote for this issue
              1 Start watching this issue