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

Refactor cache section in tempodb/backend #485

Merged
merged 12 commits into from
Feb 3, 2021

Conversation

dgzlopes
Copy link
Member

@dgzlopes dgzlopes commented Jan 28, 2021

Opened in favor of #399

What this PR does:

  • Refactors cache section in tempodb.
  • Changes /operations configs.
  • Adds Memcached section to docs.

Which issue(s) this PR fixes:
Fixes #373
Fixes #372

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@dgzlopes dgzlopes marked this pull request as ready for review February 1, 2021 20:35
@dgzlopes dgzlopes changed the title [WIP] Refactor cache section in tempodb/backend Refactor cache section in tempodb/backend Feb 1, 2021
Copy link
Contributor

@annanay25 annanay25 left a comment

Choose a reason for hiding this comment

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

Other than these small comments, everything is looking good!

tempodb/backend/cache/memcached/memcached.go Outdated Show resolved Hide resolved
tempodb/backend/cache/cache.go Outdated Show resolved Hide resolved
Copy link
Contributor

@annanay25 annanay25 left a comment

Choose a reason for hiding this comment

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

Loving this +208 −29,998 :)

tempodb/backend/cache/cache.go Outdated Show resolved Hide resolved
Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

I'll @annanay25 land the final approval and merge since he's spent more time with it, but overall excellent work. I had been frustrated with the organization of the backend for awhile and this PR is going a long way to bringing it to where I want it.

One nit, but otherwise LGTM.

Great work @dgzlopes.

tempodb/tempodb.go Show resolved Hide resolved
tempodb/tempodb.go Outdated Show resolved Hide resolved
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
Signed-off-by: Daniel González Lopes <danielgonzalezlopes@gmail.com>
@annanay25
Copy link
Contributor

Thanks @dgzlopes!

@annanay25 annanay25 merged commit fb7221c into grafana:master Feb 3, 2021
@dgzlopes dgzlopes deleted the refactor/cache-tempodb branch February 3, 2021 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants