Mike Drob commented on SOLR-9524:

RE computeIfAbsent and IOException: I recent reviewed some code and gave 
feedback on an issue pertaining to this (I can't seen to find right it now). 
The code on whatever issue it was could have theoretically used computeIfAbsent 
and would thus have been much nicer were it not for the IOException. So we 
couldn't use it; what a pain, sigh.... If we want to use it here too (were it 
not for IOException) we might want to add a utility to make this easy so that 
the semantics of what we're doing is clear. computeIfAbsent is beautiful when 
you can use it.
I've been poking at this, but due to some quirks of java generics, it's harder 
to put in a generic utility than it would appear at first blush. Going down 
this route, we may have to limit ourselves strictly to IOException.

bq. I don't think it's a big deal. Essentially it's just made caching useless. 
We didn't have any caching till now. So, it is not going to be any worse than 
what it used to be
It only made the second check useless. The first check and the put are still 
correct, so we will still have some amount of caching right now, but multiple 
threads asking at once could end up all doing the work.

> SolrIndexSearcher.getIndexFingerprint uses dubious sunchronization
> ------------------------------------------------------------------
>                 Key: SOLR-9524
>                 URL: https://issues.apache.org/jira/browse/SOLR-9524
>             Project: Solr
>          Issue Type: Bug
>      Security Level: Public(Default Security Level. Issues are Public) 
>    Affects Versions: 6.3
>            Reporter: Mike Drob
>         Attachments: SOLR-9524.patch
> In SOLR-9310 we added more code that does some fingerprint caching in 
> SolrIndexSearcher. However, the synchronization looks like it could be made 
> more efficient and may have issues with correctness.
> https://github.com/apache/lucene-solr/blob/branch_6x/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java#L2371-L2385
> Some of the issues:
> * Double checked locking needs use of volatile variables to ensure proper 
> memory semantics.
> * sync on a ConcurrentHashMap is usually a code smell

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to