magibney commented on PR #1360: URL: https://github.com/apache/solr/pull/1360#issuecomment-1433033954
The map is explicitly designed to lazily cache these `Terms` instances (accumulating them over time for a given `SolrIndexSearcher`). The returned `Terms` instances are _not_ safe for concurrent use -- considering which, I'm surprised we didn't see actual errors resulting from this usage! I suspect the correct solution will be to just skip caching the `Terms` instances altogether. We could also upgrade from Map to use a proper cache (with evictions, etc.) or some other caching strategy that effectively incorporates "creation thread" into the cache key. Not sure whether that would be worth it. It's also possible that the _way_ in which `Terms` instances returned from `SlowCompositeReaderWrapper` are used in practice actually _is_ thread-safe? In that case there might be other options, such as pulling the required thread-safe derivative of the `Terms` instance, and wrapping it in a synthetic `Terms` instance that mostly throws `UnsupportedOperationException`, and caching/returning that synthetic instance? Curious what others think here ... perhaps @dsmiley? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
