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

[HTTP/3] Implement connection scavenging #54968

Closed
geoffkizer opened this issue Jun 30, 2021 · 7 comments · Fixed by #101531
Closed

[HTTP/3] Implement connection scavenging #54968

geoffkizer opened this issue Jun 30, 2021 · 7 comments · Fixed by #101531
Assignees
Labels
area-System.Net.Http enhancement Product code improvement that does NOT require public API changes/additions in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@geoffkizer
Copy link
Contributor

We are not scavenging HTTP3 connections currently (i.e. based on idle timeout or lifetime). We need to implement this.

@geoffkizer geoffkizer added this to the 6.0.0 milestone Jun 30, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jun 30, 2021
@ghost
Copy link

ghost commented Jun 30, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

We are not scavenging HTTP3 connections currently (i.e. based on idle timeout or lifetime). We need to implement this.

Author: geoffkizer
Assignees: -
Labels:

area-System.Net.Http

Milestone: 6.0.0

@ManickaP ManickaP removed the untriaged New issue has not been triaged by the area owner label Jun 30, 2021
@karelz karelz changed the title HTTP3: Implement connection scavenging [HTTP/3] Implement connection scavenging Jul 1, 2021
@karelz karelz added the enhancement Product code improvement that does NOT require public API changes/additions label Jul 1, 2021
@karelz
Copy link
Member

karelz commented Jul 13, 2021

Triage: Would manifest as memory leak

@ManickaP
Copy link
Member

@geoffkizer What exactly is missing? I see we check the expiration and dispose and clean up the connection:

if (CheckExpirationOnGet(http3Connection) || http3Connection.Authority != authority)
{
// Connection expired.
if (NetEventSource.Log.IsEnabled()) http3Connection.Trace("Found expired HTTP3 connection.");
http3Connection.Dispose();
InvalidateHttp3Connection(http3Connection);
}

@geoffkizer
Copy link
Contributor Author

There's no HTTP3 logic here:

@geoffkizer
Copy link
Contributor Author

BTW, I think we should update the HTTP3 connection pooling logic in general to match the recent changes I made to HTTP11/2. As part of that, this should just fall out.

@ManickaP ManickaP modified the milestones: 6.0.0, Future Aug 10, 2021
@geoffkizer geoffkizer modified the milestones: Future, 7.0.0 Oct 22, 2021
@geoffkizer
Copy link
Contributor Author

I changed the milestone to 7.0. We can't deliver production quality HTTP3 without this.

@geoffkizer
Copy link
Contributor Author

See also #60729, which has some proposals for how to change how connection scavenging works.

We should decide if we want to make any of those changes before we implement scavenging for HTTP3, to avoid wasted work.

@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Apr 25, 2024
@ManickaP ManickaP self-assigned this Apr 25, 2024
@MihaZupan MihaZupan modified the milestones: Future, 9.0.0 Jun 18, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http enhancement Product code improvement that does NOT require public API changes/additions in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants