Expire values in RedisCacheAdapter #2988
Conversation
TylerBrock
force-pushed
the
redis-cache-adapter
branch
from
2d750ac to
66953ce
Compare
|
@TylerBrock updated the pull request - view changes |
| @@ -1,6 +1,8 @@ | |||
| import redis from 'redis'; | |||
| import logger from '../../logger'; | |||
|
|
|||
| const DEFAULT_REDIS_TTL = 30 * 1000; // 30 seconds | |||
There was a problem hiding this comment.
Don't think we use this here so could be removed.
Currently the schema cache TTL is passed in to ParseServer as schemaCacheTTL and then passed to the adapter with each put(). However InMemoryCache does have a default TTL if no value is passed in from the adapter, so you might want to implement?
There was a problem hiding this comment.
Arg, I had something there initially where if TTL wasn't provided we'd use one but the lockout tests wouldn't pass. I meant to remove that for the time being.
| this.client.set(key, value, function(err, res) { | ||
| resolve(); | ||
| }); | ||
| if (ttl) { |
There was a problem hiding this comment.
The logic in InMemoryCache is a little different:
TTL = 0 means store but have immediately expire, e.g. in tests.
TTL = NaN means never expire.
Your change has TTL = 0 never expiring and I'm not sure what happens when NaN is passed to psetex().
I would change to match InMemoryCache
There was a problem hiding this comment.
👍 Thank you that was the piece of information I was looking for, I'll update.
I saw this login in the InMemory cache but assumed zero meant never expire not immediately expire. That's why the tests wouldn't pass.
So I'm trying the RedisCacheAdapter and it looks like it just sets keys forever. I think they should expire, most likely after 30 seconds.