[Django] #31728: cache.backends.db._cull sometimes fails with 'NoneType' object is not subscriptable
Django
subscriptable
-----------------------------------------------+--------------------------
Reporter: knixeur | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Keywords: cache db
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------------+--------------------------
I'm sporadically getting some cache errors using database backend.
The error is: 'NoneType' object is not subscriptable
And the backtrace:
{{{
/usr/local/lib/python3.7/site-packages/django/core/handlers/base.py:143→
_get_response
/usr/local/lib/python3.7/site-packages/django/template/response.py:108→
render
/usr/local/lib/python3.7/site-packages/django/utils/decorators.py:156→
callback
/usr/local/lib/python3.7/site-packages/django/middleware/cache.py:103→
process_response
/usr/local/lib/python3.7/site-packages/django/utils/cache.py:374→
learn_cache_key
/usr/local/lib/python3.7/site-
packages/django/core/cache/backends/db.py:104→ set
/usr/local/lib/python3.7/site-
packages/django/core/cache/backends/db.py:136→ _base_set
/usr/local/lib/python3.7/site-
packages/django/core/cache/backends/db.py:277→ _cull
}}}
This is using Django 2.2.11 but I see the same code is in master.
https://github.com/django/django/blob/master/django/core/cache/backends/db.py#L270
{{{
cursor.execute(
connection.ops.cache_key_culling_sql() % table,
[cull_num])
cursor.execute("DELETE FROM %s "
"WHERE cache_key < %%s" % table,
[cursor.fetchone()[0]])
}}}
From what I can understand, {{{connection.ops.cache_key_culling_sql()}}}
command is not returning any data, so {{{cursor.fetchone()[0]}}} fails.
I guess a simple check to see if it contains data would be enough, may
apply for an easy picking.
--
Ticket URL: <https://code.djangoproject.com/ticket/31728>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
subscriptable
Reporter: Guillermo Bonvehí | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Keywords: cache db | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Guillermo Bonvehí:
Old description:
New description:
From what I can understand, the cursor after running
{{{connection.ops.cache_key_culling_sql()}}} command is not returning any
data, so {{{cursor.fetchone()[0]}}} afterwards fails.
I guess a simple check to see if it contains data would be enough, may
apply for an easy picking.
Edit: Wording
--
--
Ticket URL: <https://code.djangoproject.com/ticket/31728#comment:1>
Django
subscriptable
Reporter: Guillermo Bonvehí | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Keywords: cache db | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Guillermo Bonvehí):
Added PR https://github.com/django/django/pull/13089 from branch
https://github.com/knixeur/django/tree/ticket_31728
--
Ticket URL: <https://code.djangoproject.com/ticket/31728#comment:2>
Django
subscriptable
Reporter: Guillermo Bonvehí | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Keywords: cache db | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Guillermo Bonvehí):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/31728#comment:3>
Django
subscriptable
Reporter: Guillermo Bonvehí | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Keywords: cache db | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* needs_better_patch: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Thanks for the report and patch. Left some comments for improvements to
make the test backend agnostic, once they are addressed feel free to
uncheck the _Patch needs improvement_ flag for your patch to show up in
the review queue.
--
Ticket URL: <https://code.djangoproject.com/ticket/31728#comment:4>
Django
subscriptable
Reporter: Guillermo Bonvehí | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Keywords: cache db | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Guillermo Bonvehí):
* needs_better_patch: 1 => 0
Comment:
Thanks to you Simon for the review. I moved the test to the base test
class, if I understood correctly that's enough for it to be backend
agnostic. Let me know if there's some other change needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/31728#comment:5>
Django
subscriptable
Reporter: Guillermo Bonvehí | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: cache db | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"f386454d1302b66d0eb331ed0ae9e4811e2f3a15" f386454d]:
{{{
#!CommitTicketReference repository=""
revision="f386454d1302b66d0eb331ed0ae9e4811e2f3a15"
Fixed #31728 -- Fixed cache culling when no key is found for deletion.
DatabaseCache._cull implementation could fail if no key was found to
perform a deletion in the table. This prevented the new cache key/value
from being correctly added.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31728#comment:6>
Django
subscriptable
Reporter: Guillermo Bonvehí | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: cache db | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b2e2489d807de360068f5a34c57d5345e526caf3" b2e2489]:
{{{
#!CommitTicketReference repository=""
revision="b2e2489d807de360068f5a34c57d5345e526caf3"
[3.1.x] Fixed #31728 -- Fixed cache culling when no key is found for
deletion.
DatabaseCache._cull implementation could fail if no key was found to
perform a deletion in the table. This prevented the new cache key/value
from being correctly added.
Backport of f386454d1302b66d0eb331ed0ae9e4811e2f3a15 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31728#comment:7>
Django
subscriptable
Reporter: Guillermo Bonvehí | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: cache db | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Guillermo Bonvehí):
Thanks Mariusz! One question, I'd like to backport this to 2.2.x, should I
just open a new PR on Github and/or another ticket here?
--
Ticket URL: <https://code.djangoproject.com/ticket/31728#comment:8>
Django
subscriptable
Reporter: Guillermo Bonvehí | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: cache db | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by felixxm):
Replying to [comment:8 Guillermo Bonvehí]:
> Thanks Mariusz! One question, I'd like to backport this to 2.2.x, should
I just open a new PR on Github and/or another ticket here?
The issue has been present since the feature was introduced
(0fa1aa8711a6e1f3653e98943f2847366c0ac556). Per our backporting policy
this means it doesn't qualify for a backport to 3.0.x or 2.2.x anymore.
See https://docs.djangoproject.com/en/3.0/internals/release-process/ for
more details.
--
Ticket URL: <https://code.djangoproject.com/ticket/31728#comment:9>
Django
subscriptable
Reporter: Guillermo Bonvehí | Owner: nobody
Type: Bug | Status: closed
Component: Core (Cache system) | Version: master
Keywords: cache db | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Guillermo Bonvehí):
Thanks for the answer.
In case anyone wants to apply to their own code, here's a PR with the
changes (sorry I didn't read your answer before creating it):
https://github.com/django/django/pull/13094
--
Ticket URL: <https://code.djangoproject.com/ticket/31728#comment:10>