Skip to content

HttpConnectionPool attempts to create connections for pre-canceled requests #66990

Closed

Description

@MihaZupan

Spinning off from #66673 (comment).

Starting with 6.0, cancelling a request does not remove it from the request queue immediately.
Instead, available connections consume the queue. If they encounter an already canceled request, they simply skip it.

If a connection attempt fails, it will attempt to fail the first request in the queue if the queue hasn't moved since the connection attempt started. If the queue is not empty, it will try to inject a new connection.
This makes it so that each request gets at least one connection attempt.

When injecting a connection, the connection pool only checks that there is a request available, but not whether said request is already canceled. This results in the pool issuing a connection attempt for an already-canceled request.

If there are a lot of cancelled requests in the queue and connection attempts keep failing, the queue will be drained very slowly (we will waste a connection attempt for each entry).
It's possible this will be slower than the rate of incoming requests, resulting in the queue constantly growing with more and more cancelled requests, resulting in an effective memory leak.

This issue is worse for HTTP/2 by default since we only allow a single pending connection attempt at a time, where as we will open Min(requests, MaxConnectionsPerServer) connections for HTTP/1.1 (#66944).

Activity

  1. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Mar 22, 2022
  2. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on Mar 22, 2022
  3. removed
    untriagedNew issue has not been triaged by the area owner
    on Mar 24, 2022
  4. added this to the 6.0.x milestone on Mar 24, 2022
  5. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Mar 24, 2022
  6. MihaZupan commented on Mar 24, 2022

    @MihaZupan
    MemberAuthor

    Reopening for 6.0 servicing

  7. ghost added
    in-prThere is an active PR which will close this issue when it is merged
    on Mar 28, 2022
  8. ghost removed
    in-prThere is an active PR which will close this issue when it is merged
    on Apr 13, 2022
  9. MihaZupan commented on Apr 13, 2022

    @MihaZupan
    MemberAuthor

    Servicing PR has been merged, closing.

  10. karelz commented on Apr 21, 2022

    @karelz
    Member

    Fixed in 7.0 (main) in PR #66992 and in 6.0.5 in PR #67226.

  11. ghost locked as resolved and limited conversation to collaborators on May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions