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

Use a sorted set for unique list functionality #74

Closed
lewispb opened this issue Feb 25, 2022 · 4 comments · Fixed by #76
Closed

Use a sorted set for unique list functionality #74

lewispb opened this issue Feb 25, 2022 · 4 comments · Fixed by #76

Comments

@lewispb
Copy link
Contributor

lewispb commented Feb 25, 2022

I benchmarked an implementation of a unique list using a Redis Sorted Set as opposed to the current implementation which uses a Redis list.

https://gist.github.com/lewispb/bcb190cf72c93b67f57671e0287fa7a7

I think this shows potential performance improvements of at least 5x, or depending on the size of the list more than that.

If we want to move towards using a sorted set, we'd need to think about the migration. Maybe we maintain unique list for backwards compatiblity but add a new data structure, Kredis.sorted_set?

@dhh
Copy link
Member

dhh commented Feb 25, 2022

Think the problem is that redis already has a sorted set with certain expectations tied to it (like being unordered). Think we might have to go with something like unique_fast_list, deprecate the old one, then set a cut-off for passing over.

Although I suppose it'd be even nicer if we could detect the old data format and convert?

@lewispb
Copy link
Contributor Author

lewispb commented Feb 25, 2022

Although I suppose it'd be even nicer if we could detect the old data format and convert?

I think this would be possible actually. We could rescue the error: WRONGTYPE Operation against a key holding the wrong kind of value (Redis::CommandError). Then there'd be a one-off hit to migrate the data there and then from a list to a sorted set.

@dhh
Copy link
Member

dhh commented Feb 25, 2022

Seems worth exploring.

@lewispb
Copy link
Contributor Author

lewispb commented Feb 25, 2022

👍 I'll look into it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants