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

v1.17: quic: delay calling set_max_concurrent_uni_streams/set_receive_window (backport of #904) #967

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Apr 22, 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.


This is an automatic backport of pull request #904 done by Mergify.

Copy link
Author

mergify bot commented Apr 22, 2024

Cherry-pick of 2770424 has failed:

On branch mergify/bp/v1.17/pr-904
Your branch is up to date with 'origin/v1.17'.

You are currently cherry-picking commit 2770424782.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   streamer/src/nonblocking/quic.rs

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Copy link

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

@alessandrod can you address the merge conflict?

@codecov-commenter
Copy link

codecov-commenter commented Apr 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.6%. Comparing base (89d3014) to head (afba8f4).

Additional details and impacted files
@@           Coverage Diff           @@
##            v1.17     #967   +/-   ##
=======================================
  Coverage    81.6%    81.6%           
=======================================
  Files         806      806           
  Lines      219335   219335           
=======================================
+ Hits       179047   179104   +57     
+ Misses      40288    40231   -57     

…#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 mergify bot requested review from a team as code owners April 25, 2024 21:53
@joeaba joeaba self-requested a review April 29, 2024 23:16
Copy link

@joeaba joeaba left a comment

Choose a reason for hiding this comment

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

This pr was created before the bp enforcing rules kicked in so the groups were not added properly, I'm approving since we've got the other 2 approvals

@t-nelson
Copy link

merge me!

@t-nelson t-nelson merged commit 72d6e89 into v1.17 Apr 30, 2024
33 checks passed
@t-nelson t-nelson deleted the mergify/bp/v1.17/pr-904 branch April 30, 2024 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants