Fix missing Body.Close() calls on push to docker remote#5712
Fix missing Body.Close() calls on push to docker remote#5712estesp merged 1 commit intocontainerd:mainfrom
Conversation
|
Hi @coryb. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Build succeeded.
|
0db7e6d to
2d4261c
Compare
|
Build succeeded.
|
Discovered this while using HTTP tracing via OpenTelemetry inside of buildkitd, where the trace spans were not being reported for the registry PUT http requests. The spans are only reported on the Close for the Body, after adding these Close calls, the spans are reported as expected. Signed-off-by: coryb <cbennett@netflix.com>
2d4261c to
894b6ae
Compare
|
Build succeeded.
|
|
recheck |
|
Build succeeded.
|
|
@coryb Do you think this could be the issue in moby/buildkit#1988 (comment) or should the pool still be released internally for this case. |
|
@tonistiigi not sure, suppose it is possibly related. I don't have any data on pushes hanging, or at least we have not been running with the tracing enabled long enough to diagnose any on our side. I didn't see any delays in my testing on this specific issue anyway. |
|
@coryb Even if related it can't happen atm as the PR was reverted. But something deadlocked for some people |
|
/ok-to-test |
|
@AkihiroSuda could you "approve" the workflow to trigger CI? |
kzys
left a comment
There was a problem hiding this comment.
Looks good to me.
@containerd/committers Can someone take a look?
Discovered this while using HTTP tracing via OpenTelemetry inside of
buildkitd, where the trace spans were not being reported for the
registry PUT http requests. The spans are only reported on the Close
for the Body, after adding these Close calls, the spans are reported as
expected.
related buildkit code
cc @tonistiigi