Opened 7 years ago

Closed 6 years ago

#16708 closed Cleanup/optimization (fixed)

Session backend cached_db's exists() does not check cache before database

Reported by: jpvale@… Owned by: Aymeric Augustin
Component: contrib.sessions Version: master
Severity: Normal Keywords: session cache cached_db
Cc: jvale Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: no UI/UX: no


In the cached_db session backend, the exists() method does not check the cache before hitting the database. It should first hit the cache, and only query the database if the session is not already cached.

Attachments (2)

cached_db_exists_check_cache.diff (517 bytes) - added by jpvale@… 7 years ago.
cached_db_exists_check_cache_v2.diff (2.5 KB) - added by jvale 7 years ago.

Download all attachments as: .zip

Change History (7)

Changed 7 years ago by jpvale@…

comment:1 Changed 7 years ago by Aymeric Augustin

Needs tests: set
Triage Stage: UnreviewedDesign decision needed

This file was never modified since it was checked in at r9727.

The current behavior may be an omission that went through the reviews unnoticed, or an implementation choice: it avoids loading the contents of the session just to check if it exists. The comments on #6791 don't mention it.

Since Django's cache API doesn't provide cache.exists(key), there's no way to check if a session exists in the cache without getting the contents of the session. It's impossible to optimize this for the general case, because it depends on the speed/scalability of the database, the speed/scalability of the cache, and the size of the session.

Since sessions tend to be small, I'm +0 on this change.

If it's accepted, the patch would need tests.

Changed 7 years ago by jvale

comment:2 Changed 7 years ago by jvale

Cc: jvale added

I'm the ticket creator, thought I'd create an account here.

Added a second version of the patch, this time with tests.

comment:3 Changed 7 years ago by Alex Gaynor

Triage Stage: Design decision neededAccepted

comment:4 Changed 6 years ago by Aymeric Augustin

Owner: changed from nobody to Aymeric Augustin

comment:5 Changed 6 years ago by Aymeric Augustin

Resolution: fixed
Status: newclosed

I fixed that problem in c11f9c3193901215ec79732af6ab6c66b3c1c2ba.

Note: See TracTickets for help on using tickets.
Back to Top