Updater: fix stuck updater process #774
Conversation
|
@jzelinskie Hi, can you please take a look at this PR? It is blocking Clair from getting new vulnerability data. |
updater.go
Outdated
| // done context is used when updater finishes and all other | ||
| // go rutines in group should finish too | ||
| doneCtx := context.Background() | ||
| doneCtx, done := context.WithCancel(doneCtx) |
There was a problem hiding this comment.
Can we not wrap the existing context rather than creating a brand new unrelated one?
There was a problem hiding this comment.
@jzelinskie What do you mean by brand new context? I created a new context line above.
There was a problem hiding this comment.
Sorry for the confusion.
I'm suggesting this:
var cancel context.CancelFunc
ctx, cancel = context.WithCancel(ctx) // <-- the same context from errgroup.WithContext()There was a problem hiding this comment.
I am not sure if I understand your suggestion so I rather ask... I need 2 separate contexts to be able to distinguish between when an update is successfully done and when an update fails. You suggest merging those context together if I understand it right. How can we distinguish between those 2 cases then?
jzelinskie
added
component/worker
kind/bug
There are several goroutines in updateWhileRenewingLock() which were blocking updater process because they were waiting for event which never happened when update() function was successful. This commit adds context which unblock other goroutines when update pass.
jzelinskie
added
the
reviewed/needs rework
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Hey, this looks reasonable. I actually just haven't approved it because I didn't like the complexity building up in this part of the code, but I don't think that's a reason to block this. A follow up PR can clean things up, if it is actually needed. |
There are several goroutines in updateWhileRenewingLock() which were
blocking updater process because they were waiting for event which never
happened when update() function was successful.
This commit adds context which unblock other goroutines when update
pass.