Skip to content

Add backpressure to send buffer#97

Merged
cbrewster merged 1 commit intomainfrom
10-08-Add_backpressure_to_send_buffer
Oct 8, 2024
Merged

Add backpressure to send buffer#97
cbrewster merged 1 commit intomainfrom
10-08-Add_backpressure_to_send_buffer

Conversation

@cbrewster
Copy link
Copy Markdown
Member

Why

Currently the send buffer has no backpressure, so if a user of the river client sends too many messages too quickly, the send buffer will overflow and the client will error out.

Rather than relying on users to rate limit themselves, we introduce backpressure so client naturally will only send messages as fast as the river client/server is able to consume them.

What changed

  • Add a condvar to the message buffer to allow waiting for buffer space to come available when the buffer is full
  • Add a closed bit to the message buffer so when a session is shut down, we have a way of unblocking all the futures that are stuck waiting for space in the buffer
  • Close the message buffer when the session is closed

Test plan

  • Added a test which sends 2 * MAX_MESSAGE_BUFFER_SIZE messages to a river server. Previously this resulted in an error.
  • Added some unit tests for the MessageBuffer to ensure backpressure works and closing works

@cbrewster cbrewster requested a review from a team as a code owner October 8, 2024 18:52
@cbrewster cbrewster requested review from blast-hardcheese and removed request for a team October 8, 2024 18:52
@cbrewster
Copy link
Copy Markdown
Member Author

cbrewster commented Oct 8, 2024

Current dependencies on/for this PR:

This comment was autogenerated by Freephite.

@cbrewster cbrewster force-pushed the 10-08-Add_backpressure_to_send_buffer branch from 2a05749 to bdfe692 Compare October 8, 2024 18:55
Copy link
Copy Markdown
Collaborator

@lhchavez lhchavez left a comment

Choose a reason for hiding this comment

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

heck yeah!

Comment thread replit_river/message_buffer.py
Comment thread tests/test_message_buffer.py Outdated
Comment thread replit_river/message_buffer.py
@cbrewster cbrewster force-pushed the 10-08-Add_backpressure_to_send_buffer branch from bdfe692 to 910dd0e Compare October 8, 2024 19:14
Comment thread replit_river/session.py Outdated
@cbrewster cbrewster force-pushed the 10-08-Add_backpressure_to_send_buffer branch from 910dd0e to 6ef4509 Compare October 8, 2024 19:18
@cbrewster cbrewster merged commit 80e6d95 into main Oct 8, 2024
@cbrewster cbrewster deleted the 10-08-Add_backpressure_to_send_buffer branch October 8, 2024 19:21
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.

2 participants