Skip to content

fix(engine): use flume async API in shared MPMC channel#3385

Open
ThomsonTan wants to merge 2 commits into
open-telemetry:mainfrom
ThomsonTan:fix_mpmc_sync
Open

fix(engine): use flume async API in shared MPMC channel#3385
ThomsonTan wants to merge 2 commits into
open-telemetry:mainfrom
ThomsonTan:fix_mpmc_sync

Conversation

@ThomsonTan

Copy link
Copy Markdown
Contributor

Change Summary

The MPMC variant of the shared channel called flume's synchronous send()/recv()
from inside async fn, which parks the underlying Tokio worker thread instead of
yielding cooperatively. On a current-thread runtime where a producer and consumer
share the worker, this can deadlock; on multi-threaded runtimes it causes head-of-line
blocking and reduced throughput.

What issue does this PR close?

How are these changes tested?

Regression test added and passed locally.

Are there any user-facing changes?

Changelog

  • Added a .chloggen/*.yaml entry
  • This PR is a chore (indicated in title)
  • This is a documentation-only PR.

@ThomsonTan ThomsonTan requested a review from a team as a code owner June 29, 2026 23:02
Copilot AI review requested due to automatic review settings June 29, 2026 23:02
@github-actions github-actions Bot added lang:rust Pull requests that update Rust code area:engine Internal engine features size/M labels Jun 29, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a potential deadlock/head-of-line blocking scenario in the engine’s shared MPMC channel abstraction by switching the flume-backed variant from the synchronous send()/recv() APIs to flume’s async send_async().await / recv_async().await APIs when used inside async fn.

Changes:

  • Update SharedSender::send for the MPMC (flume) backend to use send_async(...).await.
  • Update SharedReceiver::recv for the MPMC (flume) backend to use recv_async().await.
  • Add a regression test that exercises recv polled before send on a single-thread Tokio runtime to ensure no runtime-thread blocking/deadlock (issue #1704), plus a changelog entry.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
rust/otap-dataflow/crates/engine/src/shared/message.rs Switch flume MPMC send/recv to async APIs and add a regression test proving the runtime thread is not blocked.
rust/otap-dataflow/.chloggen/engine-flume-mpmc-async-blocking.yaml Add a user-facing changelog entry documenting the engine fix for flume MPMC async behavior (issue #1704).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.23%. Comparing base (474998a) to head (4ffdfb8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3385      +/-   ##
==========================================
- Coverage   86.23%   86.23%   -0.01%     
==========================================
  Files         739      739              
  Lines      292355   292382      +27     
==========================================
+ Hits       252116   252130      +14     
- Misses      39715    39728      +13     
  Partials      524      524              
Components Coverage Δ
otap-dataflow 87.29% <96.55%> (-0.01%) ⬇️
query_engine 89.58% <ø> (ø)
otel-arrow-go 52.45% <ø> (ø)
quiver 92.19% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:engine Internal engine features lang:rust Pull requests that update Rust code size/M

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Flume MPMC should not be used with the sync interface

4 participants