Fix/try redict #569
Merged
Fix/try redict #569
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Multiple options available: - KeyDB (multithreaded open-source fork maintained by Snapchat) - Valkey (fork keeping the former BSD license) - Redict (fork switching to LGPL license) For now switch to the Redict as our needs seem to be satisfied: - they have a wide variety of containers (scratch, alpine, debian) - as it has been tested (on prod), it works smooth as a drop-in replacement - it is also present in the Fedora and EPEL repos As for the switch itself: - introduce new Kubernetes definition for Redict deployment - introduce new variables ‹with_redis› and ‹with_redict› to allow customizable deployment, in case we decide to revert, or switch - introduce ‹redis_hostname› that is deduced from the ‹with_*› variables; also »FAIL« for neither of them being set Fixes packit#561 Signed-off-by: Matej Focko <[email protected]>
Related to packit#561 Signed-off-by: Matej Focko <[email protected]>
|
Build failed. ✔️ pre-commit SUCCESS in 1m 48s |
Closed
7 tasks
nforro
approved these changes
May 2, 2024
lbarcziova
approved these changes
May 2, 2024
There was a problem hiding this comment.
thanks for writing this down!
| We have tested a seamless migration from Redis to Redict on our production | ||
| deployment. To reproduce: | ||
|
|
||
| 1. We have deployed Redict to our production cluster. |
There was a problem hiding this comment.
maybe worth adding a command how was this done (make deploy redict?)
There was a problem hiding this comment.
Now that you mention it, I forgot the check for deploying both at the same time :D
As it has been discovered¹, Redict hit some stability issues. The only common culprit appears to be spike in the memory usage, therefore raise the requested memory 2× and limit for the memory 4×. Details of the first occurrence from the issue for migrating from Redis to open-source alternative: - Smallish issue has been hit on the Thursday afternoon, first-response and fix by @nforro; appears to be caused by network flakes - _Redict logs_: Redis requested multiple times resync, failed on timeout and _Connection reset_; cause of restarts is unknown - _Sentry events_: related to the Redis sync and also failed connections to the Redict - @mfocko scaled down Redis on Monday as it doesn't appear we're hitting any blockers and will need to quick swap ¹ on at least 2 occasions Signed-off-by: Matej Focko <[email protected]>
|
Build failed. ✔️ pre-commit SUCCESS in 1m 49s |
- Create a dummy hostname for the purpose of nice debugging message - Improve the assertion to check for »exactly« one of the Redis, or Redict to be deployed - For success print the hostname that will be used. For fail print warning and hint what to change. Signed-off-by: Matej Focko <[email protected]>
Signed-off-by: Matej Focko <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.