-
Notifications
You must be signed in to change notification settings - Fork 37
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
Add guide about how to migrate from Redis #169
Add guide about how to migrate from Redis #169
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good documents. I have mostly small changes for consistency and clarity.
Two other changes that are hard to remark in the review:
- You need to fix your DCO issues. I always find this link to be helpful on fixing the DCO issues.
- looks like you checked your pycache, that will need to be removed.
12290c7
to
bf6a07c
Compare
Signed-off-by: Anastasia Alexadrova <[email protected]>
bf6a07c
to
67e680d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great document. I didn't review it completely. I just noticed some things I wanted to comment on. Feel free to ignore the comments though.
topics/migration.md
Outdated
## Why to migrate to Valkey? | ||
|
||
* Valkey is the vendor-neutral and open-source software | ||
* Enhanced performance with multi-threading, dual-channel replication and I/O threading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"multi-threading" and "I/O threading" is the same thing.
If you want to mention three things, can we mention improved memory efficiency instead?
Regarding memory efficiency, I'm thinking about two things, perhaps too complex to mention here but just FYI:
- the one-dict-per-slot in cluster mode (which also redis has, and had merged before we forked, but it's not included in any of the open source redis releases)
- embedded key in dict entry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small rendering issues but otherwise LGTM.
topics/migration.md
Outdated
|
||
a. Connect to Redis and check the number of keys: | ||
|
||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should drop the triple-backtick fencing when nesting code blocks in two level deep lists.
As written, the site renders like this
But because you can indents are also interpreted as code blocks the it still works. When I remove 54 and 59 it looks like this rendered on-site:
(You'll also need to remove this for the other alphabetical code blocks.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was surprising. There's something odd about the indentation here but I believe it should be possible to use triple backticks inside list items too.
If I read https://spec.commonmark.org/0.31.2/#list-items correctly, I believe you should indent the body of list item 2. Make
aligned under "M" in "Make" and align the body of a. Connect
under the "C" in "Connect", rather than adding 4 spaces for each level.
Example:
2. Make a backup of your Redis instance.
a. Connect to Redis and check the number of keys:
```
code block
```
or, if you want four spaces indentation per level, you can add an extra space between "2." and "Make" and between "a." and "Connect".
2. Make a backup of your Redis instance.
a. Connect to Redis and check the number of keys:
```
code block
```
3e900da
to
8b0cf82
Compare
I have tried to fix the indentation. When I build it locally it looks good for me but I cannot figure out how it renders on the website unfortunately. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me in general. I don't have time to do a full review, sorry.
Signed-off-by: Anastasia Alexadrova <[email protected]>
8b0cf82
to
34bf768
Compare
New topic page 'migration' about how to upgrade from Redis 7.2.5 for standalone and cluster.