[
https://issues.apache.org/jira/browse/LUCENE-10292?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17529668#comment-17529668
]
Chris M. Hostetter commented on LUCENE-10292:
---------------------------------------------
{quote} I'm still not sure whether these tests make sense without explicitly
stating ...
{quote}
All I was really trying to do with these tests was demonstrate that data you
get out of the Lookup before you call build(), can still be gotten from the
Lookup while build() is incrementally consuming an iterator (which may take a
long time if you are building up from a long iterator) and that this behavior
is consistent across Lookup impls (as opposed to before i filed this issue,
when most Lookups worked that way, but AnalyzingInfixSuggester would throw an
ugly exception – which was certainly confusing to users who might switch from
one impl to another).
I didn't set out to make any hard & fast guarantee about the thread safety of
all lookups – just improve the one that awas obviously inconsistent with the
others (progress, not perfection)
I'm happy to rename/re-document/remove the test code if you'd like - or confine
it to just AnalyzingInfixSuggester - but i think (assume?) we can probably
agree that the src/main code changes I've made in this issue are generally for
the better?
{quote}I think it'd be a much more user-friendly API if Lookup was actually
detached entirely from its build process (for example by replacing the current
build method with a builder() that would return a new immutable Lookup
instance). ... I'm not saying this should be implemented here - perhaps it's
worth a new issue to do this refactoring.
{quote}
+1 ... although some of the lookup impls explicitly support update()int
individual suggestions – so you'd either have to remove that functionality or
refactor the API in some way that it can still be optional.
{quote}Separately from the above, if the test fails, it'll leak threads ... It
should be replaced with a try/catch that rethrows an unchecked exception
{quote}
Yeah, that was lazy & sloppy of me – sorry about that. I'll fix it ASAP.
> AnalyzingInfixSuggester thread safety: lookup() fails during (re)build()
> ------------------------------------------------------------------------
>
> Key: LUCENE-10292
> URL: https://issues.apache.org/jira/browse/LUCENE-10292
> Project: Lucene - Core
> Issue Type: Bug
> Reporter: Chris M. Hostetter
> Assignee: Chris M. Hostetter
> Priority: Major
> Fix For: 10.0 (main), 9.2
>
> Attachments: LUCENE-10292-1.patch, LUCENE-10292-2.patch,
> LUCENE-10292-3.patch, LUCENE-10292.patch
>
>
> I'm filing this based on anecdotal information from a Solr user w/o
> experiencing it first hand (and I don't have a test case to demonstrate it)
> but based on a reading of the code the underlying problem seems self
> evident...
> With all other Lookup implementations I've examined, it is possible to call
> {{lookup()}} regardless of whether another thread is concurrently calling
> {{build()}} – in all cases I've seen, it is even possible to call
> {{lookup()}} even if {{build()}} has never been called: the result is just an
> "empty" {{List<LookupResult>}}
> Typically this is works because the {{build()}} method uses temporary
> datastructures until it's "build logic" is complete, at which point it
> atomically replaces the datastructures used by the {{lookup()}} method. In
> the case of {{AnalyzingInfixSuggester}} however, the {{build()}} method
> starts by closing & null'ing out the {{protected SearcherManager
> searcherMgr}} (which it only populates again once it's completed building up
> it's index) and then the lookup method starts with...
> {code:java}
> if (searcherMgr == null) {
> throw new IllegalStateException("suggester was not built");
> }
> {code}
> ... meaning it is unsafe to call {{AnalyzingInfixSuggester.lookup()}} in any
> situation where another thread may be calling
> {{AnalyzingInfixSuggester.build()}}
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
