Opened 13 years ago

Closed 10 years ago

#15806 closed Bug (fixed)

Error in filebased cache culling

Reported by: witek@… Owned by: Anssi Kääriäinen <akaariai@…>
Component: Core (Cache system) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In line 115 of filebased.py we have:

try:
    filelist = sorted(os.listdir(self._dir))
except (IOError, OSError):
    return

if self._cull_frequency == 0:
    doomed = filelist
else:
    doomed = [os.path.join(self._dir, k) for (i, k) in enumerate(filelist) if i % self._cull_frequency == 0]

In case of _cull_frequency == 0 instead of "doomed = filelist" we should have sth like:

doomed = [os.path.join(self._dir, d) for d in filelist]

Otherwise doomed tuple will not contains absolute paths but relative.

Attachments (3)

tests_for_15806.diff (2.4 KB ) - added by Nikolay Zakharov 13 years ago.
Regression test for filebased cache with CULL_FREQUENCY=0
patch_for_15806.diff (566 bytes ) - added by Nikolay Zakharov 13 years ago.
Initial patch
patch_with_tests_for_15806.diff (3.0 KB ) - added by Nikolay Zakharov 13 years ago.
United diff (code + tests)

Download all attachments as: .zip

Change History (12)

comment:1 by Jacob, 13 years ago

Easy pickings: unset
Triage Stage: UnreviewedAccepted

See also #15825. Not exactly related, but right around in that same part of code and probably worth fixing at the same time.

comment:2 by Nikolay Zakharov, 13 years ago

Owner: changed from nobody to Nikolay Zakharov
Status: newassigned
UI/UX: unset

by Nikolay Zakharov, 13 years ago

Attachment: tests_for_15806.diff added

Regression test for filebased cache with CULL_FREQUENCY=0

by Nikolay Zakharov, 13 years ago

Attachment: patch_for_15806.diff added

Initial patch

comment:3 by Nikolay Zakharov, 13 years ago

Has patch: set

comment:4 by Aymeric Augustin, 13 years ago

Patch needs improvement: set

Could you consolidate the tests and the fix in the same patch?

in reply to:  4 comment:5 by Nikolay Zakharov, 13 years ago

Replying to aaugustin:

Could you consolidate the tests and the fix in the same patch?

Yep, sorry for that, just done tests before actually fixing so they came up first.

You can actually pick the merged_patch.diff from #15825, it contains all in one, tests and code for #15806 and #15825.

by Nikolay Zakharov, 13 years ago

United diff (code + tests)

comment:6 by Nikolay Zakharov, 13 years ago

Patch needs improvement: unset

comment:7 by Nikolay Zakharov, 13 years ago

Owner: Nikolay Zakharov removed
Status: assignednew

comment:8 by Kevin Christopher Henry, 11 years ago

Has patch: unset
Version: 1.3master

This is fixed by the solution to #20536.

comment:9 by Anssi Kääriäinen <akaariai@…>, 10 years ago

Owner: set to Anssi Kääriäinen <akaariai@…>
Resolution: fixed
Status: newclosed

In 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.

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