[cache] Switch from memcache to redis to get rid of memjs. #928
Conversation
alloy
force-pushed
the
switch-to-redis
branch
from
9765cc3 to
2b65d79
Compare
|
Personally, I'd rather pay the 8 dollars for consistency |
|
|
||
| return client.set( | ||
| key, | ||
| JSON.stringify(data), | ||
| { expires: CACHE_LIFETIME_IN_SECONDS }, | ||
| "EX", | ||
| CACHE_LIFETIME_IN_SECONDS, |
There was a problem hiding this comment.
👍 just double-checked this interface, looks good.
| const client = redis.createClient({ | ||
| host: redisURL.hostname, | ||
| port: redisURL.port, | ||
| retryStrategy: options => { |
There was a problem hiding this comment.
in the docs this is snake case, retry_strategy.
There was a problem hiding this comment.
Good reviewing! 🕵️ At the bottom of https://github.com/NodeRedis/node_redis#usage-example they mention that all APIs/options can be either snake or camel case, so I went with camel because that‘s what we’ve been settling on for JS syntax style.
| return undefined | ||
| } | ||
| // Reconnect after: | ||
| return Math.min(options.attempt * 100, 3000) |
There was a problem hiding this comment.
this min seems redundant since we break at > 10 attempts, so the first arg will never reach over 1000. but it does allow us to vary the max attempt count without worrying i guess.
| } | ||
| // End reconnecting after a specific timeout and flush all commands with a | ||
| // individual error. | ||
| if (options.total_retry_time > 1000 * 60 * 60) { |
There was a problem hiding this comment.
this total retry time seems high... just to clarify the logic here - we're happy to stay in a reconnecting state for up to an hour, but every 10 attempts we flush the command buffer with errors (by returning undefined below)?
There was a problem hiding this comment.
Also unclear on this. In the constructor, I would opt to fail hard and bail out on the process if we can't establish a connection.
There was a problem hiding this comment.
Correct. I’m open to suggestions for a better time, this is just the suggested default value from their README.
There was a problem hiding this comment.
@izakp This is not just for the initial connection but for any situation where the connection was lost.
There was a problem hiding this comment.
Defaults seem like a good place to start to me (but I was also surprised about the hour).
There was a problem hiding this comment.
👍 yeah i'm ok with this, perhaps a little complex for a default strategy. It would be nice to not terminate the process just because the cache is missing (one should be able to run an app slowly without caches). But that is perhaps an idealised view of caching ;)
|
@cavvia Good questions regarding the retry strategy. I should have noted that I simply copied the suggested defaults over, which appears to be the way to do things nowadays with this client. None of the values are written in stone, so if y’all have suggestions for improvements then that’s a-ok with me 👍 |
| } | ||
| // End reconnecting after a specific timeout and flush all commands with a | ||
| // individual error. | ||
| if (options.total_retry_time > 1000 * 60 * 60) { |
There was a problem hiding this comment.
Also unclear on this. In the constructor, I would opt to fail hard and bail out on the process if we can't establish a connection.
|
Add-ons are highway robbery! But yeah, seems like a small openredis add-on makes sense for staging. Feel free to merge when this is set up in staging/prod! |
Closes #869.
This switches us back to Redis to fix the issue of the memos client getting into a state where it can’t reconnect to the service anymore. Hopefully this also reduces some back pressure during high load times, but at the very least remove noise that makes it harder to diagnose performance issues.
Of the various available Heroku addons I for now chose openredis, as they are the cheapest and I think we already use that in other apps (?).
We currently pay $300/5GB for memcache, the available options with openredis are $200/3.4GB or $400/5.6GB. I guess we’ll have to shell out the extra $100 to get at a minimum the same amount we have now?
Unfortunately openredis does not have a free option that we could use in staging, the smallest is $8. Would you rather have we pay a little to use the same service-provider in staging or use one of the other redis addons that provide a free option?