Opened 5 years ago

Closed 4 years ago

#16858 closed Bug (duplicate)

incr() on locmem cache resets the expiry time

Reported by: Malcolm Box Owned by: Berker Peksag
Component: Core (Cache system) Version: 1.5-rc-1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Alex Gaynor)

On the locmem cache backend, calling incr() causes the key's timeout value to be reset to the default for the cache.

To reproduce:

  • setup the default cache to be locmem and with a timeout of 1 second
    >> cache.set('key', 10, 300)
    >> time.sleep(1)
    >> cache.get('key')
    10
    >> cache.incr('key')
    >> time.sleep(1)
    >> cache.get('key')
    None
    

The cache should still contain the key at the last line.

The problem comes from the implementation of incr() in base.py which uses get() and set() to implement incr(). Since set() is called without a timeout, the cache default time is used.

Attachments (2)

locmem-patch.diff (1004 bytes) - added by Malcolm Box 5 years ago.
Patch for locmem cache to bring incr() in line with memcached behaviour
locmem_incr_expiration_16858.diff (2.2 KB) - added by Michael Manfre 5 years ago.
locmem cache's incr() will no longer change timeout to mimic memcached

Download all attachments as: .zip

Change History (11)

comment:1 Changed 5 years ago by Alex Gaynor

Description: modified (diff)

I'm assuming you consider this a bug, and would like it to not reset the expiry time ;) What's the justification for that behavior, memcached does it?

comment:2 Changed 5 years ago by Malcolm Box

Yes, I consider this a bug.

As far as I can tell, memcached does not alter the expiry time of a key when incr()/decr() is called on it. Running the same test using a memcached cache backend, I get the value out at the end.

It seems highly counter-intuitive that updating a key's value should suddenly reset the timeout and cause the key to expire.

comment:3 Changed 5 years ago by Alex Gaynor

Triage Stage: UnreviewedAccepted

Well, I think the locmem's behavior is a bit more intuitive personally, but we've generally tried to make the "stupid" backends do what memcached would do, since they're often used in development/testing. Marking as accepted.

Changed 5 years ago by Malcolm Box

Attachment: locmem-patch.diff added

Patch for locmem cache to bring incr() in line with memcached behaviour

comment:4 Changed 5 years ago by Malcolm Box

Easy pickings: set
Has patch: set

comment:5 Changed 5 years ago by Chris Beaven

Needs tests: set

Changed 5 years ago by Michael Manfre

locmem cache's incr() will no longer change timeout to mimic memcached

comment:6 Changed 5 years ago by Michael Manfre

Needs tests: unset

The original patch walked the expiration time forward by time.time() with each call to incr() and decr(). The new patch uses a special timeout value to skip setting the expiration time.

comment:7 Changed 4 years ago by paper82

Owner: changed from nobody to paper82

comment:8 Changed 4 years ago by Berker Peksag

Owner: changed from paper82 to Berker Peksag
Status: newassigned
Version: 1.31.5-rc-1

comment:9 Changed 4 years ago by Berker Peksag

Resolution: duplicate
Status: assignedclosed
Note: See TracTickets for help on using tickets.
Back to Top