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

Version the caches and increase version when bugs are fixed #1625

Closed
colega opened this issue Apr 4, 2022 · 4 comments · Fixed by #1631
Closed

Version the caches and increase version when bugs are fixed #1625

colega opened this issue Apr 4, 2022 · 4 comments · Fixed by #1631

Comments

@colega
Copy link
Contributor

colega commented Apr 4, 2022

Is your feature request related to a problem? Please describe.

A customer complained about issues they were seeing in cached data, and after analysing it for a while, we found that #1542 was the cause for bad response being cached.

Even though the bug was fixed, the cached data stayed there for a while so the results were not reflecting the fix.

Describe the solution you'd like

Wrap our caches with something that would add a version to the keys, like this:

package cache

import (
	"context"
	"fmt"
	"strings"
	"time"
)

type Versioned struct {
	cache         Cache
	versionPrefix string
}

func NewVersioned(c Cache, version uint) Versioned {
	return Versioned{
		cache:         c,
		versionPrefix: fmt.Sprintf("%d@", version),
	}
}

func (c Versioned) Store(ctx context.Context, data map[string][]byte, ttl time.Duration) {
	versioned := make(map[string][]byte, len(data))
	for k, v := range data {
		versioned[c.addVersion(k)] = v
	}
	c.cache.Store(ctx, versioned, ttl)
}

func (c Versioned) Fetch(ctx context.Context, keys []string) map[string][]byte {
	versionedKeys := make([]string, len(keys))
	for i, k := range keys {
		versionedKeys[i] = c.addVersion(k)
	}
	versionedRes := c.Fetch(ctx, versionedKeys)
	res := make(map[string][]byte, len(versionedRes))
	for k, v := range versionedRes {
		res[c.removeVersion(k)] = v
	}
	return res
}

func (c Versioned) Name() string {
	return c.cache.Name()
}

func (c Versioned) addVersion(k string) string {
	return c.versionPrefix + k
}
func (c Versioned) removeVersion(k string) string {
	return strings.TrimPrefix(k, c.versionPrefix)
}

Then increase the version with which it's instantiated every time a bug is fixed.

Describe alternatives you've considered

Add version to each one of the cache usages instead, like here:

http://github.com/grafana/mimir/blob/f33cdfda71d99dc653842454389a04590fcb979e/pkg/frontend/querymiddleware/split_and_cache.go#L302-L313

@pstibrany
Copy link
Member

Instead of adding versioning to our caches (what would the version be based on? Our internal release number? Mimir version? Commit hash? Manually when fixing a bug as you suggested?), Mimir could also flush the cache after rollout. For results cache with only a handful of query-frontends, that may be good-enough solution.

@colega
Copy link
Contributor Author

colega commented Apr 5, 2022

What would the version be based on? Our internal release number? Mimir version? Commit hash? Manually when fixing a bug as you suggested?

I would increase the version each time a bug is fixed (or a features is introduced).

There's no need to flush the cache in most of the releases, so I wouldn't do that.

@pracucci
Copy link
Collaborator

pracucci commented Apr 5, 2022

I think add version to the cache key makes sense. I also agree that it should be done only after a bug fix or a backward incompatible feature is introduced, not just at every rollout.

@colega
Copy link
Contributor Author

colega commented Apr 5, 2022

This is my proposal: #1631

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants