[ 
https://issues.apache.org/jira/browse/LUCENE-6370?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14382164#comment-14382164
 ] 

Dave Borowitz commented on LUCENE-6370:
---------------------------------------

I didn't take refcounting into account, that's a good point. We haven't run 
this patch in production, so there might be some issues. But we do have a 
pretty good integration test suite that depends heavily on NRT searching.

Basically what I did in the original Gerrit patch I linked to was implement my 
own ReferenceManager<T> that was a fork of SearcherManager. Is there a way I 
should tweak that fork that would avoid the refcounting problems you 
anticipate? (Extending SearcherManager is not an option unfortunately as that 
class is final.)

Do you think the refcounting problems would show up in a simple test? We could 
add a NRT case to TestUninvertingReader without too much work, I think.

> UninvertingReader cannot be used with ControlledRealTimeReopenThread
> --------------------------------------------------------------------
>
>                 Key: LUCENE-6370
>                 URL: https://issues.apache.org/jira/browse/LUCENE-6370
>             Project: Lucene - Core
>          Issue Type: Bug
>    Affects Versions: 5.0
>            Reporter: Dave Borowitz
>         Attachments: LUCENE-6370.patch
>
>
> In order to sort over non-DocValues fields in 5.0 we need to use an 
> UninvertingReader to get the old FieldCache behavior back. However, 
> UninvertingReader cannot be (easily) used with a 
> ControlledRealTimeReopenThread.
> Specifically, the easiest way to construct a ControlledRealTimeReopenThread 
> is with a SearcherManager. The only way I found to wire an UninvertingReader 
> into SearcherManager is to implement a SearcherFactory that  wraps the 
> passed-in reader. Unfortunately, that runs afoul of the check in 
> SearcherManager.getSearcher that requires "SearcherFactory must wrap exactly 
> the provided reader". So, as long as this check is there, I simply don't see 
> a way to use UninvertingReader with NRT functionality.
> I think this is a serious issue for programs that need to be able to use NRT 
> search features on indexes created with previous Lucene versions, for whom 
> upgrading the index is not an easy option. If they were previously relying on 
> sorting implicitly via FieldCache, the _only_ ways to upgrade are:
> a) rebuild the index using DocValues fields, or
> b) use UninvertingReader
> Right now there's a catch-22, as (a) is assumed to be not an option and (b) 
> is broken due to this bug.
> I have a hacky workaround for Gerrit Code Review up for review here:
> https://gerrit-review.googlesource.com/#/c/66613/6/gerrit-lucene/src/main/java/com/google/gerrit/lucene/WrappableSearcherManager.java@191
> Basically, it loosens the restriction on the newSearcher result to allow 
> Filtered{Directory,Leaf}Readers that wrap the original reader. This appears 
> to work fine for us, and I don't see anything in UninvertingReader that would 
> cause me to believe it doesn't work. However, I'm no expert on Lucene 
> internals and I don't know why that identity check was there in the first 
> place, so I may be missing something.
> Please do not take that patch directly until I have gotten permission from my 
> employer to contribute it.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to