redis/redis-io

document that getset kills key expirations

mlissner opened this issue · 4 comments

(Sorry this is an issue and not a PR; I couldn't find where the getset documentation code is.)

I just learned today, over in freelawproject/courtlistener#1675 that getset nukes ttl. Here's some python code to demonstrate (r is a connection to redis):

In [38]: r.set('asdf', "foo")
Out[38]: True

In [39]: r.expire('asdf', 60)
Out[39]: True

In [40]: r.ttl('asdf')
Out[40]: 55

In [41]: r.ttl('asdf')
Out[41]: 53

In [42]: r.getset('asdf', 'bar')
Out[42]: 'foo'

In [43]: r.ttl('asdf')
Out[43]: -1

I know getset is now deprecated, but this comes as a very big surprise to me, and I'm even more surprised it's not documented. I've noticed that SET now has a KEEPTTL parameter, so apparently this is something that has come up before.

Would it be a good idea to put a note about this in the getset docs? Something like:

If a key already exists and has a time to live, the TTL will be reset.

I think that's all it would take and that might help others like me in the future.

Thank everybody for the wonderful thing that is redis. What a lovely tool.

Hello @mlissner - thanks for bringing this up.

I couldn't find where the getset documentation code is

https://github.com/redis/redis-doc/blob/master/commands/getset.md

I know getset is now deprecated

Indeed, it is deprecated in favor of the all-in-one evolved SET command. That said, "deprecated" means very little here because I don't see us removing it from the project in the foreseeable future :)

so apparently this is something that has come up before

If you read the docs about SET, you'll find that it resets TTL (and doesn't care about the TYPE of the key if it exists). The KEEPTTL option was added recently to modify this behavior. Because GETSET also does a SET, it is implied that it also resets the TTL.

Would it be a good idea to put a note about this

Sure. Please make the PR that clarifies this and close this issue.

Just created redis/redis-doc#1570 by copying from the SET documentation. Thanks for the guidance.

Looks like this small change to the docs got dropped. Any chance we can merge it?

Merged. Thanks @mlissner! This issue can be closed.