Code

Opened 6 years ago

Closed 6 years ago

Last modified 2 years ago

#7639 closed Uncategorized (fixed)

Locmem is not multiprocess nor threadsafe

Reported by: permon Owned by: nobody
Component: Core (Cache system) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Documentation says that this cache backend is safe in multiprocess environment and also threadsafe. This probably not true.
a) multiprocess - it's complete nonsense - every process maintains its own cache object. So key found in c1 is not found in c2, also invalidation is impossible.
b) threads - problem is almost same - there is implementation of threadsafe writing/reading, but objects in threads are different so caches are different.

Solution:
a) Don't bother with this backend and say in documentation that is good only for development server or one htread/process servers, than it's completely ok to remove locking code.
b) Do it at least thread-safe (make some singleton object?)

Attachments (1)

0001-Clarify-LocMemCache-being-per-process-Refs-7639.diff (796 bytes) - added by projectgus 2 years ago.
Possible docs patch for LocMemCache "multi-process"

Download all attachments as: .zip

Change History (6)

comment:1 Changed 6 years ago by mtredinnick

  • milestone 1.0 deleted
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

For the multiprocess case, invalidation is the only possible issue here. The fact that there are separate caches for separate processes doesn't make it unsafe. A cache doesn't change the results, it just possible speeds things up. So if something is in the cache for one process and not for another, that shouldn't be a real issue. If it is, that object isn't cacheable in the first place (so what if one person sees a blog post 60 seconds before somebody else? It doesn't change anything). But invalidation might be a concern there.

(Removing 1.0 milestone, since that hasn't been determined yet)

comment:2 Changed 6 years ago by ericholscher

  • milestone set to 1.0 maybe
  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 6 years ago by jacob

  • Resolution set to fixed
  • Status changed from new to closed

The locmem cache is perfectly safe it's just not very good -- each process and thread gets its own cache, so there's no chances of conflicts.

I've added this note to the docs; you'll see it when the doc-refactor lands:

Note that each process will have its own private cache instance, which means no
cross-process caching is possible. This obviously also means the local memory
cache isn't particularly memory-efficient, so it's probably not a good choice
for production environments.

comment:4 Changed 6 years ago by anonymous

  • milestone 1.0 maybe deleted

Milestone 1.0 maybe deleted

comment:5 Changed 2 years ago by projectgus

  • Easy pickings unset
  • Severity set to Normal
  • Type set to Uncategorized
  • UI/UX unset

Would it please be possible to change the phrase "This cache is multi-process" to "per-process" (will attach possible patch.) Per-process seems to be a more accurate description of how the cache works.

I realise the subsequent paragraph explains it perfectly (thanks!), but there must be other idiot newbies like me who skimmed the docs before making a new admin command, thought "great, multi-process, no need to change anything!" and then wondered why their cache didn't invalidate cleanly. Before I, of course, RTed-rest-of-the-FM. :)

Changed 2 years ago by projectgus

Possible docs patch for LocMemCache "multi-process"

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.