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

query-frontend: Allow separate label cache #3560

Merged
merged 1 commit into from
Dec 10, 2020

Conversation

craigfurman
Copy link
Contributor

@craigfurman craigfurman commented Dec 10, 2020

Instead of using the cache config from the query-range.

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Fixed a bug in which the cache config from the query frontend's query range cache was passed to the label cache. This made it impossible to use a label cache on its own, because invalid empty yaml (from the query range cache) would be passed to the label cache config builder.

Any users who are attempting to use separate label and query range caches today should find that the query range one is being used for both. If this is merged and upgraded to, it will cause the label cache to become used instead.

Verification

I've only done some very basic testing, by running the thanos binary locally and verifying whether the different config combinations cause an error on startup with or without this fix.

The error I see from master is:

{"caller":"main.go:131","err":"response cache with type  is not supported\ngithub.com/thanos-io/thanos/pkg/queryfrontend.NewCacheConfig\n\t/go/src/github.com/thanos-io/thanos/pkg/queryfrontend/config.go:139\nmain.runQueryFrontend\n\t/go/src/github.com/thanos-io/thanos/cmd/thanos/query_frontend.go:161\nmain.registerQueryFrontend.func1\n\t/go/src/github.com/thanos-io/thanos/cmd/thanos/query_frontend.go:129\nmain.main\n\t/go/src/github.com/thanos-io/thanos/cmd/thanos/main.go:129\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:204\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1374\ninitializing the labels cache config\nmain.runQueryFrontend\n\t/go/src/github.com/thanos-io/thanos/cmd/thanos/query_frontend.go:163\nmain.registerQueryFrontend.func1\n\t/go/src/github.com/thanos-io/thanos/cmd/thanos/query_frontend.go:129\nmain.main\n\t/go/src/github.com/thanos-io/thanos/cmd/thanos/main.go:129\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:204\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1374\npreparing query-frontend command failed\nmain.main\n\t/go/src/github.com/thanos-io/thanos/cmd/thanos/main.go:131\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:204\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1374","level":"error","ts":"2020-12-10T11:20:32.8017198Z"}

Instead of using the cache config from the query-range.

Signed-off-by: Craig Furman <[email protected]>
@craigfurman craigfurman force-pushed the qfe-label-memcached-config branch from 505fd48 to 3626813 Compare December 10, 2020 12:23
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the fix.

Copy link
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@GiedriusS GiedriusS merged commit 92d1bb9 into thanos-io:master Dec 10, 2020
@craigfurman craigfurman deleted the qfe-label-memcached-config branch December 11, 2020 09:36
@craigfurman
Copy link
Contributor Author

Thanks! Any chance this can make it into a patch release soon? I'm not sure what thanos' release cadence is.

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 this pull request may close these issues.

3 participants