Skip to content

fix(bes): keep the stream alive through long replays and slow drains - #1344

Merged
cristifalcas merged 2 commits into
mainfrom
cf/bes-sink-backpressure
Sep 2, 2026
Merged

fix(bes): keep the stream alive through long replays and slow drains#1344
cristifalcas merged 2 commits into
mainfrom
cf/bes-sink-backpressure

Conversation

@cristifalcas

@cristifalcas cristifalcas commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Prevent BES uploads from timing out while making progress:

  • Drain acknowledgements already available during buffered-event replay, avoiding HTTP/2 flow-control stalls on long replays.
  • Extend the half-close timeout after each acknowledgement, so it measures acknowledgement silence rather than total backlog-drain time.

visible to end-users: yes
docs: no
breaking change: no
release notes: yes

@aspect-workflows

aspect-workflows Bot commented Jul 25, 2026

Copy link
Copy Markdown

Aspect Workflows Tasks

📅 Wed Sep 2 02:12:13 UTC 2026

Task Results


⏱ Last updated Wed Sep 2 02:21:25 UTC 2026 · 📊 GitHub API quota 0/15,000 (0% used, resets in 59m)
🚀 Powered by Aspect CLI (v0.0.0-dev)  |  Aspect Build · X · LinkedIn · YouTube

Comment thread crates/axl-runtime/src/engine/bazel/sink/grpc.rs Outdated
@cristifalcas
cristifalcas marked this pull request as ready for review July 25, 2026 15:47

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8d16622162

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread crates/axl-runtime/src/engine/bazel/sink/grpc.rs Outdated
Comment thread crates/axl-runtime/src/engine/bazel/sink/grpc.rs Outdated
@thesayyn

thesayyn commented Aug 4, 2026

Copy link
Copy Markdown
Member

This has some conflicts.

@cristifalcas
cristifalcas force-pushed the cf/bes-sink-backpressure branch from 8d16622 to e3cf514 Compare August 8, 2026 14:52
A reconnect replays every retained event before resuming the live stream.
That loop never reads the response side, so on a large buffer the server's
flow-control window fills, it stops reading requests, and the replay trips
`send_stall_timeout` — reconnecting into the same wall each time. Acks are
now drained as they arrive and applied once the loop releases its borrow of
the buffer; the handful of extra replayed events that costs are deduped by
sequence number server-side.

Separately, half-close held a flat 30s to drain whatever was outstanding.
That deadline is a budget for *silence*, not for how long a drain may take:
a build that ends holding a large unacked backlog, against a backend acking
steadily but slower than 30s, gets its stream torn down and fully replayed
at the end of the build. It is now pushed out on every ack. Draining to
empty already exits, so only a backend that has actually gone quiet spends
it, and the 30s bound against a silent one is unchanged.

Both paths got more exposed with #1353, which replaced the 10,000-event cap
with a 256 MiB byte budget — the replay these guard is now up to two orders
of magnitude larger.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cristifalcas
cristifalcas force-pushed the cf/bes-sink-backpressure branch from c44a27e to 072de45 Compare August 8, 2026 15:08
@cristifalcas cristifalcas changed the title fix(bes): backpressure the sink instead of dying when acks fall behind fix(bes): keep the stream alive through long replays and slow drains Aug 8, 2026
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: b1511bd2-4fd2-46da-bdff-63cd90168b77

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gregmagolan gregmagolan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm

@cristifalcas
cristifalcas merged commit 5635ba8 into main Sep 2, 2026
67 of 71 checks passed
@cristifalcas
cristifalcas deleted the cf/bes-sink-backpressure branch September 2, 2026 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants