Opened 13 years ago
Closed 13 years ago
#16708 closed Cleanup/optimization (fixed)
Session backend cached_db's exists() does not check cache before database
| Reported by: | Owned by: | Aymeric Augustin | |
|---|---|---|---|
| Component: | contrib.sessions | Version: | dev |
| 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 |
Description
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)
Change History (7)
by , 13 years ago
| Attachment: | cached_db_exists_check_cache.diff added |
|---|
comment:1 by , 13 years ago
| Needs tests: | set |
|---|---|
| Triage Stage: | Unreviewed → Design decision needed |
by , 13 years ago
| Attachment: | cached_db_exists_check_cache_v2.diff added |
|---|
comment:2 by , 13 years ago
| Cc: | 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 by , 13 years ago
| Triage Stage: | Design decision needed → Accepted |
|---|
comment:4 by , 13 years ago
| Owner: | changed from to |
|---|
comment:5 by , 13 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
I fixed that problem in c11f9c3193901215ec79732af6ab6c66b3c1c2ba.
Note:
See TracTickets
for help on using tickets.
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.