Re: [Django] #15825: File-based caching doesn't cull truly randomly

[Django]

#15825: File-based caching doesn't cull truly randomly
-------------------------------------+-------------------------------------
Reporter: gkuenning | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: Core (Cache system)
Version: SVN | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by desh):

* has_patch: 0 => 1
* ui_ux: => 0

Comment:

I've added patch against trunk AND patch that also fixes #15806 (it's
actually just a result of merge) to avoid merging conflicts.

I've removed sorted() cause it's make no sense with random culling. I've
also replaced nested for that traverse directories to delete paths by
shutils.rmtree, as it does the same.

Tests changed in two ways: "cull%d" became "cull-%d" and initial inserts
count for cull test was reduced from 49 to 34, to have 34 unique 2-char
prefixes after hashing. It's actually a bit rude but in fact, #14250 was
about as rude as this.

I see two other ways to solve last problem with tests:
* change length of directory name from 2 to 3, so there will be 4096
possible prefixes (conceptually, same hack but from different pov);
* change the way of culling: delete exactly
_num_entries//_cull_frequency FILES, not directories (bad thing, cause it
will not scale with big cache sizes).

Waiting for review.

--
Ticket URL: <https://code.djangoproject.com/ticket/15825#comment:4>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#15825: File-based caching doesn't cull truly randomly
-------------------------------------+------------------------------------

Reporter: gkuenning | Owner: nobody
Type: Bug | Status: new

Component: Core (Cache system) | Version: master

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------

Comment (by anonymous):

This seems to have been forgotten. Has the patch rotted?

--
Ticket URL: <https://code.djangoproject.com/ticket/15825#comment:5>

[Django]

#15825: File-based caching doesn't cull truly randomly
-------------------------------------+------------------------------------
Reporter: gkuenning | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------

Comment (by susan):

Just ran the latest patch "merge_patch" against the cache tests; they
passed. Then, I ran the applied patch against the full test suite, which
produced a failure. Stack trace here:

FAIL: test_cull (cache.tests.FileBasedCacheTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "../django/tests/cache/tests.py", line 1085, in test_cull
self.perform_cull_test(35, 24)
File "../django/tests/cache/tests.py", line 495, in perform_cull_test
self.assertEqual(count, final_count)
AssertionError: 23 != 24

--
Ticket URL: <https://code.djangoproject.com/ticket/15825#comment:6>

[Django]

#15825: File-based caching doesn't cull truly randomly
-------------------------------------+------------------------------------
Reporter: gkuenning | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------

Changes (by jaap3):

* cc: jaap3 (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/15825#comment:7>

[Django]

#15825: File-based caching doesn't cull truly randomly
-------------------------------------+------------------------------------
Reporter: gkuenning | Owner: nobody
Type: Bug | Status: new
Component: Core (Cache system) | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------

Comment (by jaap3):

Created a pull request that addresses this issue:
https://github.com/django/django/pull/1514 (also #20536)

--
Ticket URL: <https://code.djangoproject.com/ticket/15825#comment:8>

[Django]

#15825: File-based caching doesn't cull truly randomly
-------------------------------------+------------------------------------
Reporter: gkuenning | Owner: nobody

Type: Bug | Status: closed

Component: Core (Cache system) | Version: master

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+------------------------------------

Changes (by Anssi Kääriäinen <akaariai@…>):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"7be638390e18fcbfaaed638f9908673360c280d3"]:
{{{
#!CommitTicketReference repository=""
revision="7be638390e18fcbfaaed638f9908673360c280d3"
Fixed #20536 -- rewrite of the file based cache backend

* Safer for use in multiprocess environments
* Better random culling
* Cache files use less disk space
* Safer delete behavior

Also fixed #15806, fixed #15825.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/15825#comment:9>