Skip to content
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

quic: delay calling set_max_concurrent_uni_streams/set_receive_window #904

Merged
merged 2 commits into from
Apr 22, 2024

Conversation

alessandrod
Copy link

@alessandrod alessandrod commented Apr 19, 2024

Delay calling connection.set_max_concurrent_uni_streams() and connection.set_receive_window() to when we know we've accepted the connection. Avoids a short window where the peer could start transmitting even if we're going to drop the connection, and avoids taking the connection mutex and waking a task.

…oing to drop a connection

Avoids taking a mutex and waking a task.
@alessandrod alessandrod force-pushed the quic-uni-streams-later branch from 01c457a to d5a73f9 Compare April 19, 2024 04:00
@alessandrod alessandrod changed the title quic: delay calling connection.set_max_concurrent_uni_streams quic: delay calling set_max_concurrent_uni_streams/set_receive_window Apr 19, 2024
@alessandrod alessandrod requested a review from lijunwangs April 19, 2024 04:04
Copy link

mergify bot commented Apr 19, 2024

Backports to the stable branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule.

Copy link

mergify bot commented Apr 19, 2024

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.9%. Comparing base (86667f5) to head (4a23d31).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master     #904     +/-   ##
=========================================
- Coverage    81.9%    81.9%   -0.1%     
=========================================
  Files         853      853             
  Lines      231776   231779      +3     
=========================================
- Hits       189871   189854     -17     
- Misses      41905    41925     +20     

Copy link

@lijunwangs lijunwangs left a comment

Choose a reason for hiding this comment

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

Looks good

@lijunwangs lijunwangs merged commit 2770424 into anza-xyz:master Apr 22, 2024
40 checks passed
mergify bot pushed a commit that referenced this pull request Apr 22, 2024
…#904)

* quic: don't call connection.set_max_concurrent_uni_streams if we're going to drop a connection

Avoids taking a mutex and waking a task.

* quic: don't increase the receive window before we've actually accepted a connection

(cherry picked from commit 2770424)

# Conflicts:
#	streamer/src/nonblocking/quic.rs
mergify bot pushed a commit that referenced this pull request Apr 22, 2024
…#904)

* quic: don't call connection.set_max_concurrent_uni_streams if we're going to drop a connection

Avoids taking a mutex and waking a task.

* quic: don't increase the receive window before we've actually accepted a connection

(cherry picked from commit 2770424)
alessandrod added a commit that referenced this pull request Apr 24, 2024
…#904)

* quic: don't call connection.set_max_concurrent_uni_streams if we're going to drop a connection

Avoids taking a mutex and waking a task.

* quic: don't increase the receive window before we've actually accepted a connection

(cherry picked from commit 2770424)

# Conflicts:
#	streamer/src/nonblocking/quic.rs
lijunwangs pushed a commit that referenced this pull request Apr 24, 2024
…#904)

* quic: don't call connection.set_max_concurrent_uni_streams if we're going to drop a connection

Avoids taking a mutex and waking a task.

* quic: don't increase the receive window before we've actually accepted a connection

(cherry picked from commit 2770424)
lijunwangs pushed a commit that referenced this pull request Apr 24, 2024
…_window (backport of #904) (#968)

quic: delay calling set_max_concurrent_uni_streams/set_receive_window (#904)

* quic: don't call connection.set_max_concurrent_uni_streams if we're going to drop a connection

Avoids taking a mutex and waking a task.

* quic: don't increase the receive window before we've actually accepted a connection

(cherry picked from commit 2770424)

Co-authored-by: Alessandro Decina <[email protected]>
alessandrod added a commit that referenced this pull request Apr 24, 2024
…#904)

* quic: don't call connection.set_max_concurrent_uni_streams if we're going to drop a connection

Avoids taking a mutex and waking a task.

* quic: don't increase the receive window before we've actually accepted a connection

(cherry picked from commit 2770424)

# Conflicts:
#	streamer/src/nonblocking/quic.rs
alessandrod added a commit that referenced this pull request Apr 25, 2024
…#904)

* quic: don't call connection.set_max_concurrent_uni_streams if we're going to drop a connection

Avoids taking a mutex and waking a task.

* quic: don't increase the receive window before we've actually accepted a connection

(cherry picked from commit 2770424)

# Conflicts:
#	streamer/src/nonblocking/quic.rs
t-nelson pushed a commit that referenced this pull request Apr 30, 2024
…_window (backport of #904) (#967)

* quic: delay calling set_max_concurrent_uni_streams/set_receive_window (#904)

* quic: don't call connection.set_max_concurrent_uni_streams if we're going to drop a connection

Avoids taking a mutex and waking a task.

* quic: don't increase the receive window before we've actually accepted a connection

(cherry picked from commit 2770424)

# Conflicts:
#	streamer/src/nonblocking/quic.rs

* Fix merge conflicts

---------

Co-authored-by: Alessandro Decina <[email protected]>
anwayde pushed a commit to firedancer-io/agave that referenced this pull request Jul 23, 2024
…_window (backport of anza-xyz#904) (anza-xyz#968)

quic: delay calling set_max_concurrent_uni_streams/set_receive_window (anza-xyz#904)

* quic: don't call connection.set_max_concurrent_uni_streams if we're going to drop a connection

Avoids taking a mutex and waking a task.

* quic: don't increase the receive window before we've actually accepted a connection

(cherry picked from commit 2770424)

Co-authored-by: Alessandro Decina <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants