zhaih commented on PR #13124: URL: https://github.com/apache/lucene/pull/13124#issuecomment-1958716375
+1 to move executor away from the Codec API (altho it's me who placed them there LOL) > it would be nice to fully encapsulate the merging concurrency there instead of having two sources of merging concurrency that are not aware of one another. I like the idea, but I'm a little bit worry about the situation where the HNSW merging threads are taking most of the resource such that original CMS threads cannot be executed. As the `numWorkers` we configured is a per-merge limit, which means if there are several big merges happening at the same time, say `n` merges, they will use `n * (numWorkers + 1)` threads, the thread pool can run out of threads very fastly and result in smaller merges be blocked by large merges. So I think it is still better to have two separate thread pools for inter-segment merge and inner segment merge, but I wonder whether we can have a `ThreadPoolManager` which configures both inter and inner pool. And CMS and HNSW merge (and any future merges) will source from this manager? Also I heard there's a usecase where they're currently sharing one CMS with multiple IW in a same process for better resource management, this new `ThreadPoolManager` can be a solution for them as well maybe? -- 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
