sohami commented on issue #12347: URL: https://github.com/apache/lucene/issues/12347#issuecomment-1584939394
Thanks @jpountz and @javanna for the discussion. I also agree that we should have either of `executor` or `SliceExecutor`. However, given there are already constructors accepting `executor` adding constructor to only accept `SliceExecutor` will break the consumers using `IndexSearcher(IndexReader r, Executor executor)` and passing `null` for `executor`. Thinking out loud I feel those usage is also incorrect as those consumers should just use `IndexSearcher(IndexReader r)` instead. I am not sure if this will be fine considering the behavior can still remain same which means it will still be backward compatible ? > I agree that executing slices on either the caller thread or the executor makes it difficult to reason about what happens where and to appropriately size thread pools. Though to have a clear distinction between a coordinator thread pool and a collection thread pool, sequential execution should also be offloaded to the executor which is somehow controversial. I am a bit nervous about making this the default, hence the thought on exposing a SliceExecutor abstraction that is configurable and can make these decisions I agree here that exposing `SliceExecutor` is giving the extension opportunity to customize it based on their use case and default implementation can remain as is. > Could it be an idea to move some portions of IndexSearcher#search(Query, CollectorManager) to the SliceExecutor itself, for it to deal with task creation and execution as a whole? I am not sure on this as currently `SliceExecutor` doesn't know anything about details of the `tasks` passed to it and just controls how the tasks needs to be executed. I think task creation should still remain in `IndexSearcher` otherwise we may end up making `SliceExecutor` aware about internals like `Collector` or `CollectorManager` -- 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...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
