[net] http2: add wrapped ClientConn

0 views
Skip to first unread message

Damien Neil (Gerrit)

unread,
Apr 28, 2026, 12:29:16 PM (17 hours ago) Apr 28
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Damien Neil voted Commit-Queue+1

Commit-Queue+1
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: net
Gerrit-Branch: master
Gerrit-Change-Id: I5245620594538ab9befad888823ed40c6a6a6964
Gerrit-Change-Number: 771142
Gerrit-PatchSet: 1
Gerrit-Owner: Damien Neil <dn...@google.com>
Gerrit-Reviewer: Damien Neil <dn...@google.com>
Gerrit-Comment-Date: Tue, 28 Apr 2026 16:29:12 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
unsatisfied_requirement
satisfied_requirement
open
diffy

Nicholas Husin (Gerrit)

unread,
Apr 28, 2026, 4:47:24 PM (12 hours ago) Apr 28
to Damien Neil, goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Tom Bergan, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Damien Neil and Tom Bergan

Nicholas Husin voted and added 1 comment

Votes added by Nicholas Husin

Code-Review+2

1 comment

File http2/transport_wrap.go
Line 320, Patchset 1 (Latest): StreamsPending: 0,
Nicholas Husin . unresolved

Is my understanding correct that:

  • We cannot really give any other number here since we have no visibility into `net/http/internal/http2.ClientConn,pendingRequests`?
  • The only situation where this is problematic is that if someone uses `StrictMaxConcurrentStreams == true`, they can unknowingly create a lot of pending streams in the underlying `net/http/internal/http2` implementation while thinking that all is well since `StreamsPending` returned by the x/net wrapper is always 0?

If so, worth adding comments / docs?

Open in Gerrit

Related details

Attention is currently required from:
  • Damien Neil
  • Tom Bergan
Submit Requirements:
  • requirement satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: net
Gerrit-Branch: master
Gerrit-Change-Id: I5245620594538ab9befad888823ed40c6a6a6964
Gerrit-Change-Number: 771142
Gerrit-PatchSet: 1
Gerrit-Owner: Damien Neil <dn...@google.com>
Gerrit-Reviewer: Damien Neil <dn...@google.com>
Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
Gerrit-Reviewer: Tom Bergan <tomb...@google.com>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-Attention: Tom Bergan <tomb...@google.com>
Gerrit-Attention: Damien Neil <dn...@google.com>
Gerrit-Comment-Date: Tue, 28 Apr 2026 20:47:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Damien Neil (Gerrit)

unread,
Apr 28, 2026, 6:17:38 PM (11 hours ago) Apr 28
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention needed from Damien Neil, Nicholas Husin, Nicholas Husin and Tom Bergan

Damien Neil uploaded new patchset

Damien Neil uploaded patch set #2 to this change.
Following approvals got outdated and were removed:
Open in Gerrit

Related details

Attention is currently required from:
  • Damien Neil
  • Nicholas Husin
  • Nicholas Husin
  • Tom Bergan
Submit Requirements:
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
  • requirement is not satisfiedTryBots-Pass
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: newpatchset
Gerrit-Project: net
Gerrit-Branch: master
Gerrit-Change-Id: I5245620594538ab9befad888823ed40c6a6a6964
Gerrit-Change-Number: 771142
Gerrit-PatchSet: 2
Gerrit-Owner: Damien Neil <dn...@google.com>
Gerrit-Reviewer: Damien Neil <dn...@google.com>
Gerrit-Reviewer: Nicholas Husin <hu...@google.com>
Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
Gerrit-Reviewer: Tom Bergan <tomb...@google.com>
Gerrit-CC: Gopher Robot <go...@golang.org>
Gerrit-Attention: Tom Bergan <tomb...@google.com>
Gerrit-Attention: Nicholas Husin <hu...@google.com>
Gerrit-Attention: Damien Neil <dn...@google.com>
Gerrit-Attention: Nicholas Husin <n...@golang.org>
unsatisfied_requirement
open
diffy

Damien Neil (Gerrit)

unread,
Apr 28, 2026, 6:23:33 PM (11 hours ago) Apr 28
to goph...@pubsubhelper.golang.org, Nicholas Husin, Nicholas Husin, golang...@luci-project-accounts.iam.gserviceaccount.com, Tom Bergan, Gopher Robot, golang-co...@googlegroups.com
Attention needed from Nicholas Husin, Nicholas Husin and Tom Bergan

Damien Neil voted and added 1 comment

Votes added by Damien Neil

Commit-Queue+1

1 comment

File http2/transport_wrap.go
Line 320, Patchset 1: StreamsPending: 0,
Nicholas Husin . resolved

Is my understanding correct that:

  • We cannot really give any other number here since we have no visibility into `net/http/internal/http2.ClientConn,pendingRequests`?
  • The only situation where this is problematic is that if someone uses `StrictMaxConcurrentStreams == true`, they can unknowingly create a lot of pending streams in the underlying `net/http/internal/http2` implementation while thinking that all is well since `StreamsPending` returned by the x/net wrapper is always 0?

If so, worth adding comments / docs?

Damien Neil

Good catch; I think I meant to go back and think about pending requests, and forgot.

Added support for StreamsPending: When starting a RoundTrip, we try to reserve a concurrency slot (if none was already reserved) and account the request as pending if this fails.

We decrement the pending count as requests complete, which should converge on a correct value.

Open in Gerrit

Related details

Attention is currently required from:
  • Nicholas Husin
  • Nicholas Husin
  • Tom Bergan
Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: net
    Gerrit-Branch: master
    Gerrit-Change-Id: I5245620594538ab9befad888823ed40c6a6a6964
    Gerrit-Change-Number: 771142
    Gerrit-PatchSet: 2
    Gerrit-Owner: Damien Neil <dn...@google.com>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-Reviewer: Nicholas Husin <hu...@google.com>
    Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
    Gerrit-Reviewer: Tom Bergan <tomb...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Tom Bergan <tomb...@google.com>
    Gerrit-Attention: Nicholas Husin <hu...@google.com>
    Gerrit-Attention: Nicholas Husin <n...@golang.org>
    Gerrit-Comment-Date: Tue, 28 Apr 2026 22:23:29 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Nicholas Husin <n...@golang.org>
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Damien Neil (Gerrit)

    unread,
    Apr 28, 2026, 6:53:09 PM (10 hours ago) Apr 28
    to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
    Attention needed from Damien Neil, Nicholas Husin, Nicholas Husin and Tom Bergan

    Damien Neil uploaded new patchset

    Damien Neil uploaded patch set #3 to this change.
    Following approvals got outdated and were removed:
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Damien Neil
    • Nicholas Husin
    • Nicholas Husin
    • Tom Bergan
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: newpatchset
    Gerrit-Project: net
    Gerrit-Branch: master
    Gerrit-Change-Id: I5245620594538ab9befad888823ed40c6a6a6964
    Gerrit-Change-Number: 771142
    Gerrit-PatchSet: 3
    Gerrit-Owner: Damien Neil <dn...@google.com>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-Reviewer: Nicholas Husin <hu...@google.com>
    Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
    Gerrit-Reviewer: Tom Bergan <tomb...@google.com>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Tom Bergan <tomb...@google.com>
    Gerrit-Attention: Nicholas Husin <hu...@google.com>
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Damien Neil (Gerrit)

    unread,
    Apr 28, 2026, 6:53:34 PM (10 hours ago) Apr 28
    to goph...@pubsubhelper.golang.org, Tom Bergan, golang...@luci-project-accounts.iam.gserviceaccount.com, Nicholas Husin, Nicholas Husin, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Damien Neil, Nicholas Husin and Nicholas Husin

    Damien Neil removed Tom Bergan from this change

    Deleted Reviewers:
    • Tom Bergan
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Damien Neil
    • Nicholas Husin
    • Nicholas Husin
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: deleteReviewer
    Gerrit-Project: net
    Gerrit-Branch: master
    Gerrit-Change-Id: I5245620594538ab9befad888823ed40c6a6a6964
    Gerrit-Change-Number: 771142
    Gerrit-PatchSet: 3
    Gerrit-Owner: Damien Neil <dn...@google.com>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-Reviewer: Nicholas Husin <hu...@google.com>
    Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy

    Damien Neil (Gerrit)

    unread,
    Apr 28, 2026, 6:53:45 PM (10 hours ago) Apr 28
    to goph...@pubsubhelper.golang.org, golang...@luci-project-accounts.iam.gserviceaccount.com, Nicholas Husin, Nicholas Husin, Gopher Robot, golang-co...@googlegroups.com
    Attention needed from Nicholas Husin and Nicholas Husin

    Damien Neil voted Commit-Queue+1

    Commit-Queue+1
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Nicholas Husin
    • Nicholas Husin
    Submit Requirements:
    • requirement is not satisfiedCode-Review
    • requirement satisfiedNo-Unresolved-Comments
    • requirement is not satisfiedReview-Enforcement
    • requirement is not satisfiedTryBots-Pass
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: net
    Gerrit-Branch: master
    Gerrit-Change-Id: I5245620594538ab9befad888823ed40c6a6a6964
    Gerrit-Change-Number: 771142
    Gerrit-PatchSet: 3
    Gerrit-Owner: Damien Neil <dn...@google.com>
    Gerrit-Reviewer: Damien Neil <dn...@google.com>
    Gerrit-Reviewer: Nicholas Husin <hu...@google.com>
    Gerrit-Reviewer: Nicholas Husin <n...@golang.org>
    Gerrit-CC: Gopher Robot <go...@golang.org>
    Gerrit-Attention: Nicholas Husin <hu...@google.com>
    Gerrit-Attention: Nicholas Husin <n...@golang.org>
    Gerrit-Comment-Date: Tue, 28 Apr 2026 22:53:42 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    unsatisfied_requirement
    satisfied_requirement
    open
    diffy
    Reply all
    Reply to author
    Forward
    0 new messages