[adapters] Warn when the pipeline stalls due to output buffers filling.#5170
[adapters] Warn when the pipeline stalls due to output buffers filling.#5170
Conversation
I tested this manually. It seems like a lot of work to test it automatically. Signed-off-by: Ben Pfaff <blp@feldera.com>
blp
added
connectors
There was a problem hiding this comment.
Pull request overview
This PR adds warning functionality to detect and log when the pipeline stalls due to output buffers being full, helping operators identify backpressure issues.
Key Changes
- Added
next_warning()method toLongOperationWarningto calculate when the next warning should occur - Implemented progressive warning logic in the circuit thread's main loop that logs messages when output buffers remain full for extended periods
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/adapters/src/util.rs | Added helper method to calculate next warning deadline |
| crates/adapters/src/controller.rs | Integrated backpressure warning tracking in the circuit thread loop |
ryzhyk
left a comment
ryzhyk
left a comment
There was a problem hiding this comment.
this is great, thank you!
| .get_or_insert_with(|| LongOperationWarning::new(Duration::from_secs(1))); | ||
| warning.check(|elapsed| { | ||
| info!( | ||
| "pipeline stalled {} seconds because output buffers are full", |
There was a problem hiding this comment.
To make the error message more actionable, would it be possible to add some more user feedback like:
"pipeline stalled {} seconds because output buffers from connector {connector-name} are full (observed output throughput is {} records/second). Pleases tune the output connector or downstream data destination for higher throughputs."
There was a problem hiding this comment.
Yes, this is absolutely possible, and it would be a better message than what I added. But it would also take me the rest of the day to do, instead of an hour this morning. It's a compromise between insight and implementation time.
There was a problem hiding this comment.
Maybe merge this and add an issue (possibly even a meta issue) that collects instances like this.
There was a problem hiding this comment.
Maybe merge this and add an issue (possibly even a meta issue) that collects instances like this.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
I tested this manually. It seems like a lot of work to test it automatically.