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.

Sign up for GitHub

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

Jump to bottom

chore(kuma-cp) reduce cache key size #2214

Merged
merged 3 commits into from
Jun 28, 2021
Merged

chore(kuma-cp) reduce cache key size #2214

merged 3 commits into from
Jun 28, 2021

Conversation

nhamlh
Copy link
Contributor

@nhamlh nhamlh commented Jun 25, 2021 •
edited
Loading

Summary

GetCLA() generates quite big keys for its cache store. In my case with 117 sidecars it
takes nearly 2000 chars length of each key.
This change fixes that by hashing the key first.

Full changelog

  • [Implement ...]
  • [Fix ...]

Issues resolved

Fix #XXX

Documentation

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

nhamlh added 3 commits June 25, 2021 10:35
@nhamlh
chore(kuma-cp) reduce key size of cache
8a84be5 
GetCLA() generates quite big keys for its cache store. In my case it
takes nearly 2000 chars length of each key.
This change fixes that by hashing the key first.

Signed-off-by: Nham Le <lehoainham@gmail.com>
@nhamlh
fix(kuma-cp) panicc intermittently
2a7ad05 
Fix kumahq#2201

Signed-off-by: Nham Le <lehoainham@gmail.com>
@nhamlh
Merge branch 'master' into chore-reduce-cache-key-size
56beff1
@nhamlh nhamlh requested a review from a team as a code owner June 25, 2021 03:42
lobkovilya
lobkovilya requested changes Jun 25, 2021
View reviewed changes
Copy link
Contributor

@lobkovilya lobkovilya left a comment

Choose a reason for hiding this comment

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

The solution we have with long keys implies the absence of collisions, that's why we don't have to resolve them. The solution you propose makes sense, but collisions should be properly handled in that case. Otherwise, it could be just a mess

@nhamlh
Copy link
Contributor Author

nhamlh commented Jun 25, 2021

@lobkovilya thanks for your comment. I chose sha256 instead of sha1 md5 because of avoiding collision like you said.

@nhamlh nhamlh changed the title Chore reduce cache key size chore(kuma-cp) reduce cache key size Jun 25, 2021
@lobkovilya lobkovilya merged commit 776523f into kumahq:master Jun 28, 2021
@lobkovilya lobkovilya mentioned this pull request Jun 28, 2021
Merged
5 tasks
mergify bot pushed a commit that referenced this pull request Jun 28, 2021
@nhamlh
chore(kuma-cp) reduce cache key size (#2214)
ef5d527 
(cherry picked from commit 776523f)
@mergify mergify bot mentioned this pull request Jun 28, 2021
Merged
lobkovilya pushed a commit that referenced this pull request Jun 28, 2021
@mergify
chore(kuma-cp) reduce cache key size (#2214) (#2231)
353150b
@nhamlh nhamlh deleted the chore-reduce-cache-key-size branch May 31, 2022 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lobkovilya lobkovilya lobkovilya approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nhamlh @lobkovilya