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

change from node_redis to ioredis #21

Closed
wants to merge 1 commit into from

Conversation

thelinuxlich
Copy link

Two tests failing, if you have some time to spare please review it

@LewisJEllis
Copy link
Member

I just realized I wrote up a big response to this a while back, and forgot to hit enter. Fortunately the tab was still open, so here we are - apologies for the delay!

I looked into both libraries a bit more and considered a few things.

First, performance. It looks like node_redis recently did a bit of a perf overhaul and activity on the project has generally picked up. I read through some performance comparison discussion here and ultimately decided to run the benchmarks myself. I found both locally and on a digitalocean instance that node_redis is not significantly, or even strictly, faster than ioredis.

Next, weight/dependencies. I prefer to minimize the dependency tree; node_redis depends only on a deque implementation (which has no further dependencies), while ioredis depends on that and a few more. ioredis's dependency on lodash in particular adds a lot of weight:

$ du -sh node_modules/ioredis
3.2M    node_modules/ioredis
$ du -sh node_modules/redis
612K    node_modules/redis
$ du -sh node_modules/ioredis/node_modules/lodash
2.1M    node_modules/ioredis/node_modules/lodash

Similarly, the added memory usage to node of require('ioredis') is ~5x that of require('redis'). I wish ioredis tried to be a bit lighter, and all else equal, node_redis would win on this point alone. But this difference isn't a dealbreaker since the absolute numbers are still small, and all else is not equal since node_redis doesn't have cluster support.

Having given it more consideration, I'm on board with migrating to ioredis and will look into doing so in the next few days. I'll start by moving the work you've done here onto a branch and then looking into whatever else needs fixing from there. I'll aim to have it done by the end of the weekend, if not sooner.

@matteocontrini
Copy link

Any progress?

@LewisJEllis
Copy link
Member

I sort of lagged on this for a while, but with my holiday break starting later today I should have time to make some long-awaited progress! I have a nice todo log of features that I'm quite excited to get in and this is at the top of that list.

@matteocontrini
Copy link

I can't wait to see the new features 😄

@hustcer
Copy link

hustcer commented Aug 24, 2016

ioredis@2.3.0 is much smaller:

du -sh node_modules/ioredis/
488K    node_modules/ioredis/

@skeggse
Copy link
Member

skeggse commented Aug 9, 2017

What's the latest on ioredis vs redis? It sounds like the two are on a long-term collision course, and that performance is comparable, but I'm not certain. I don't have a strong opinion one way or the other, but this PR would definitely need to be updated for 1.0.0. Feel free to reopen if you think this is still worthwhile.

@skeggse skeggse closed this Aug 9, 2017
@thelinuxlich
Copy link
Author

The problem is not the performance (although ioredis is faster), It is the cluster support

@skeggse
Copy link
Member

skeggse commented Aug 9, 2017

Ah, apologies.

@skeggse skeggse reopened this Aug 9, 2017
skeggse pushed a commit that referenced this pull request Aug 11, 2017
This allows client sharing between multiple Queue instances, or other uses.

It also begins to add support for ioredis (cc #16 and #21). We might consider supporting both, though there are some major inconsistencies between the two libraries, such as the multi-exec callback form.
skeggse pushed a commit that referenced this pull request Aug 11, 2017
This allows client sharing between multiple Queue instances, or other uses.

It also begins to add support for ioredis (cc #16 and #21). We might consider supporting both, though there are some major inconsistencies between the two libraries, such as the multi-exec callback form.
skeggse pushed a commit that referenced this pull request Aug 11, 2017
This allows client sharing between multiple Queue instances, or other uses.

It also begins to add support for ioredis (cc #16 and #21). We might consider supporting both, though there are some major inconsistencies between the two libraries, such as the multi-exec callback form.
@compwright compwright closed this Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants