-
Notifications
You must be signed in to change notification settings - Fork 594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OCPBUGS-29479: Add retries to async cache initialization #13610
Changes from 3 commits
975cee7
6e153d8
1a23fc3
9761d5f
5f03015
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,10 @@ import ( | |
"k8s.io/klog" | ||
) | ||
|
||
const ( | ||
initializationRetryDelay = 30 * time.Second | ||
) | ||
|
||
type cachingFuncType[T any] func(ctx context.Context) (T, error) | ||
|
||
type AsyncCache[T any] struct { | ||
|
@@ -27,13 +31,21 @@ func NewAsyncCache[T any](ctx context.Context, reloadPeriod time.Duration, cachi | |
cachingFunc: cachingFunc, | ||
} | ||
|
||
item, err := cachingFunc(ctx) | ||
var err error | ||
ctx, cancel := context.WithTimeout(ctx, 5*time.Minute) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would expect something in terms of seconds, not minutes 👀 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is intentionally long in order to give the API server time to become available. This context gets canceled on the first successful |
||
wait.UntilWithContext(ctx, func(ctx context.Context) { | ||
TheRealJon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
item, err := cachingFunc(ctx) | ||
if err != nil { | ||
klog.V(4).Infof("failed to setup an async cache - retrying in %s", initializationRetryDelay) | ||
return | ||
} | ||
c.cachedItem = item | ||
cancel() | ||
}, initializationRetryDelay) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the delay const is used in place of a check interval here |
||
|
||
if err != nil { | ||
return nil, fmt.Errorf("failed to setup an async cache - caching func returned error: %w", err) | ||
} | ||
|
||
c.cachedItem = item | ||
|
||
return c, nil | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you wan to be under the 150 liveness probe delay, which I think means you want this delay to 11 seconds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See openshift/console-operator#869 which changes the probes for the console container.