-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Asynchronous FlightServerChannel to avoid thread contention per request #19403
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
Asynchronous FlightServerChannel to avoid thread contention per request #19403
Conversation
5bb19d9
to
278bf33
Compare
278bf33
to
24384a2
Compare
❌ Gradle check result for 24384a2: 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? |
❌ Gradle check result for 24384a2: 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? |
@andrross It would great if you can review this one, thank you |
24384a2
to
0fc1213
Compare
❌ Gradle check result for 0fc1213: 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? |
❌ Gradle check result for 0fc1213: 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? |
plugins/arrow-flight-rpc/src/main/java/org/opensearch/arrow/flight/transport/BatchTask.java
Outdated
Show resolved
Hide resolved
❌ Gradle check result for 6a90b41: 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? |
❌ Gradle check result for 6a90b41: 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? |
❌ Gradle check result for 6a90b41: null 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? |
❌ Gradle check result for 6a90b41: 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? |
6a90b41
to
6fe1cb7
Compare
I rebased from main to get the latest test fixes |
❌ Gradle check result for 6fe1cb7: 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? |
❌ Gradle check result for 6fe1cb7: 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? |
❌ Gradle check result for 6fe1cb7: 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? |
❌ Gradle check result for 6fe1cb7: 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? |
Signed-off-by: Rishabh Maurya <[email protected]>
Signed-off-by: Rishabh Maurya <[email protected]>
6fe1cb7
to
40b621b
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #19403 +/- ##
============================================
- Coverage 72.90% 72.89% -0.02%
+ Complexity 69967 69922 -45
============================================
Files 5676 5676
Lines 321143 321224 +81
Branches 46429 46441 +12
============================================
+ Hits 234138 234159 +21
- Misses 68092 68140 +48
- Partials 18913 18925 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5e6ee97
into
opensearch-project:main
Description
Addresses contention per request when multiple threads are flushing the response to the same
FlightServerChannel
. This can happen in search path when concurrent segment search is enabled where multiple threads will try to flush corresponding batches to theFlightServerChannel
. This can cause thread to be blocked as it is currently protected under synchronized block.Solution
The better way to handle it is to process the flushing of response, lets call it
BatchTask
, asynchronously. This can be done by having single threaded executor per request, which can keep addingBatchTask
from multiple threads for the given request to its queue and process them linearly.FlightServerChannel
in a round robin way when a request is received at the server. This guarantees linear processing.FlightServerChannel
i.e. regular batch flush, complete stream and error are handled asynchronously now.FlightTransportChannel
to theBatchTask
to release it when the stream is either completed or results in an error, which was not the case before and could be released in the finally block.Related Issues
Resolves #[Issue number to be closed when this PR is merged]
Check List
- [ ] API changes companion pull request created, if applicable.- [ ] Public documentation issue/PR created, if applicable.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.