javanna commented on issue #15605: URL: https://github.com/apache/lucene/issues/15605#issuecomment-4358830104
I am catching up on this, and I read the comments above, but I don't follow entirely what the usecase is to call `newCollector` concurrently. In the design of inter-segment concurrency in Lucene, `newCollector` gets called sequentially by the thread "coordinating" the search, which lines up one collector per slice (group of segments) and then forks to a different thread per collector instance created in the first place to perform the concurrent search. The thread safety needs to be guaranteed for all data structures that are shared across the different collectors instances, often used for early termination (as well as e.g. hits counting). There is no need for `newCollector` to be thread safe in this flow. Do we have a clear scenario where users may want to call `newCollector` directly and concurrently? I am not sure that this is supported, and we certainly don't test that. Furthermore, it seems that the only reason that we have shared internal state holding a list of collectors is to support a `getCollectors` method that is now unused. I am not sure what that is used for by users. Do we have a code example of that? If so, it would be good to confirm that that usecase is supported and add tests for it, otherwise, deprecate and remove that method which would in turn remove the need for the shared internal state that prompted the creation of this issue. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
