-
Notifications
You must be signed in to change notification settings - Fork 2.4k
[Draft] Optionally add virtual threads for search and index_searcher threadpools #20446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
[Draft] Optionally add virtual threads for search and index_searcher threadpools #20446
Conversation
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
Signed-off-by: Peter Alfonsi <petealft@amazon.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
❌ Gradle check result for 7f776e4: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
@peteralfonsi I think we had many conversation regarding VTs (in OpenSearch and Lucene as well) in the past and general conclusion is that search threads use file I/O (reading segments) and JDK fileystem access is mostly blocking (JEP [1] also explicitly mentions that, VT are not helpful there). To your point, as far I know, OpenJDK does not support io_uring at the moment, so what are the benefits and where they are coming from? Thank you. |
|
@reta This is intended to go alongside an io_uring PR from @kaushalmahi12 (he's on vacation this week so he likely won't post it until next Monday, but his branch does have a functioning implementation). With both changes combined, we see pretty significant speedups of ~50% or so on various big5 OSB operations. I'm not sure about OpenJDK not supporting io_uring - maybe Kaushal or @abiesps could speak more to that? |
|
Yes, this is correct @reta . In our past experiments we were able to validate that virtual threads alone doesn't provide a lot of benefits with regular file I/O in Java. Thats why we will use this along with iouring based File IO. |
Description
This PR adds an ExecutorService implementation which shares the logic of
ResizableExecutorBuilder, but uses a virtual thread factory instead. If a feature flag is enabled, the search and index_searcher threadpools will use this implementation. When paired with io_uring, this lets us yield threads which are waiting for IO to complete, improving concurrency.The PR is still in a draft state. In particular, before merging I want to make more things dynamic (at least the threadpool max size count, and perhaps even whether we use virtual threads at all if this is possible), and most likely change the logic of the multiplier setting to instead set the default number of search and index_searcher threads to something like num_cpu + io_concurrency, and reduce the queue size from 1000 accordingly such that the sum of the max thread count and the queue size remains unchanged.
We will also want to expose virtual thread scheduling stats from VirtualThreadSchedulerMXBean in some API, but this is currently in another branch (main...peteralfonsi:OpenSearch:vthreads-api) as it may require some nasty reflection to get it to work and I want to see if we can do it more cleanly.
I also have some test results (both functional and performance) which I will attach here before marking this as ready.
Related Issues
Resolves #18840, part of larger issue #18833
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.