Skip to content

cluster: Renew the context after communicating with the registry#31586

Merged
vieux merged 1 commit intomoby:masterfrom
aaronlehmann:digest-pin-context
Mar 14, 2017
Merged

cluster: Renew the context after communicating with the registry#31586
vieux merged 1 commit intomoby:masterfrom
aaronlehmann:digest-pin-context

Conversation

@aaronlehmann
Copy link

@aaronlehmann aaronlehmann commented Mar 7, 2017

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.

Fixes #31427

cc @nishanttotla

We might want to backport this to a stable release.

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]>
@nishanttotla
Copy link
Contributor

Code LGTM

I'll try to find a way to test this PR

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

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

@thaJeztah
Copy link
Member

We might want to backport this to a stable release.

If we need this into 17.03, can you prepare a cherry-pick?

@aaronlehmann aaronlehmann deleted the digest-pin-context branch March 15, 2017 16:53
@aaronlehmann
Copy link
Author

Do you mean I should open a PR against 17.03.x?

@thaJeztah
Copy link
Member

@aaronlehmann yes, if you have time; I think there's another PR that needs to go into 17.03.1, so possibly there's gonna be an rc2; if not, we can always close the PR

@aaronlehmann
Copy link
Author

Opened #31861

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