Skip to content

[17.03.2] cluster: Renew the context after communicating with the registry#33117

Merged
thaJeztah merged 1 commit intomoby:17.03.xfrom
aaronlehmann:digest-pin-context-17.03
May 10, 2017
Merged

[17.03.2] cluster: Renew the context after communicating with the registry#33117
thaJeztah merged 1 commit intomoby:17.03.xfrom
aaronlehmann:digest-pin-context-17.03

Conversation

@aaronlehmann
Copy link

Cherry-pick of #31586

When pinning by digest, the registry might be slow or unresponsive. This
could cause the context to already be expired by the time UpdateService
or CreateService is called. We want digest pinning to be a best-effort
operation, so it's problematic if a slow or misbehaving registry
prevents the service operation from completing. Replace the context
after communicating with the registry, so we have a fresh timeout for
the gRPC call.

cc @thaJeztah

When pinning by digest, the registry might be slow or unresponsive. This
could cause the context to already be expired by the time UpdateService
or CreateService is called. We want digest pinning to be a best-effort
operation, so it's problematic if a slow or misbehaving registry
prevents the service operation from completing. Replace the context
after communicating with the registry, so we have a fresh timeout for
the gRPC call.

Signed-off-by: Aaron Lehmann <[email protected]>
(cherry picked from commit f8273a2)
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@mlaventure
Copy link
Contributor

z errors look real to me, but they don't seem related to this PR 🤷‍♂️

@thaJeztah
Copy link
Member

All green now - merging.

Thanks @aaronlehmann !

@thaJeztah thaJeztah merged commit 7f394b8 into moby:17.03.x May 10, 2017
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.

6 participants