Skip to content

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

Closed
aaronlehmann wants to merge 1 commit intomoby:17.03.xfrom
aaronlehmann:digest-pin-context-17.03
Closed

[17.03.2] cluster: Renew the context after communicating with the registry#31861
aaronlehmann wants to merge 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.

@aaronlehmann aaronlehmann added this to the 17.03.1 milestone Mar 15, 2017
@thaJeztah thaJeztah changed the title cluster: Renew the context after communicating with the registry [17.03.1] cluster: Renew the context after communicating with the registry Mar 15, 2017
@tiborvass
Copy link
Contributor

@aaronlehmann please update changelog then.

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)
@aaronlehmann aaronlehmann force-pushed the digest-pin-context-17.03 branch from 6c1dadc to 161b2a5 Compare March 15, 2017 18:01
@aaronlehmann
Copy link
Author

@tiborvass: done

@tiborvass
Copy link
Contributor

LGTM

@tiborvass
Copy link
Contributor

Ping @vieux to decide

@thaJeztah
Copy link
Member

ping @vieux - did you discuss with @aaronlehmann ?

@vieux
Copy link
Contributor

vieux commented Mar 21, 2017

Let's wait for a .2 if it ever happen to include this one.

@vieux vieux closed this Mar 21, 2017
@aaronlehmann
Copy link
Author

In that case, why close the PR? Wouldn't it be better to merge it to the branch so we don't forget it if we do a .2?

@tiborvass
Copy link
Contributor

@vieux Agree with @aaronlehmann we can still merge it.

@thaJeztah thaJeztah changed the title [17.03.1] cluster: Renew the context after communicating with the registry [17.03.2] cluster: Renew the context after communicating with the registry May 5, 2017
@thaJeztah thaJeztah modified the milestones: 17.03.2, 17.03.1 May 5, 2017
@thaJeztah
Copy link
Member

Looks like I'm not able to reopen this one, so if we decide on 17.03.2, we'll need a new PR unfortunately

@aaronlehmann
Copy link
Author

re-opened: #33117

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.

5 participants