Skip to content
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

Send watch bookmarks every minute #90249

Merged
merged 1 commit into from Apr 24, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 19 additions & 9 deletions staging/src/k8s.io/apiserver/pkg/storage/cacher/cacher.go
Expand Up @@ -193,10 +193,7 @@ func newTimeBucketWatchers(clock clock.Clock) *watcherBookmarkTimeBuckets {
// adds a watcher to the bucket, if the deadline is before the start, it will be
// added to the first one.
func (t *watcherBookmarkTimeBuckets) addWatcher(w *cacheWatcher) bool {
nextTime, ok := w.nextBookmarkTime(t.clock.Now())
if !ok {
return false
}
nextTime := w.nextBookmarkTime(t.clock.Now())
bucketID := nextTime.Unix()
t.lock.Lock()
defer t.lock.Unlock()
Expand Down Expand Up @@ -1240,13 +1237,26 @@ func (c *cacheWatcher) add(event *watchCacheEvent, timer *time.Timer) bool {
}
}

func (c *cacheWatcher) nextBookmarkTime(now time.Time) (time.Time, bool) {
// For now we return 2s before deadline (and maybe +infinity is now already passed this time)
// but it gives us extensibility for the future(false when deadline is not set).
// bookmarkHeartbeatFrequency defines how frequently bookmarks should be
// under regular conditions.
const bookmarkHeartbeatFrequency = time.Minute

func (c *cacheWatcher) nextBookmarkTime(now time.Time) time.Time {
// We try to send bookmarks:
// (a) roughly every minute
// (b) right before the watcher timeout - for now we simply set it 2s before
// the deadline
// The former gives us periodicity if the watch breaks due to unexpected
// conditions, the later ensures that on timeout the watcher is as close to
// now as possible - this covers 99% of cases.
heartbeatTime := now.Add(bookmarkHeartbeatFrequency)
if c.deadline.IsZero() {
Copy link
Contributor

Choose a reason for hiding this comment

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

QQ: Is it possible (watch without deadline/timeout) or is it just a safe check?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you're using our framework like reflector - it will always be set:
https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/client-go/tools/cache/reflector.go#L387
We also try to default it it is't not provided:
https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go#L249

But you may configure it in a way to not provide minRequestTimeout, and then it may happen (though in properly configuder custer it shouldn't happen).

return c.deadline, false
return heartbeatTime
}
if pretimeoutTime := c.deadline.Add(-2 * time.Second); pretimeoutTime.Before(heartbeatTime) {
return pretimeoutTime
}
return c.deadline.Add(-2 * time.Second), true
return heartbeatTime
}

func getEventObject(object runtime.Object) runtime.Object {
Expand Down