template access hierarchy is broken

Bug #265912 reported by Ppsys
2
Affects Status Importance Assigned to Milestone
GNU Mailman
Fix Released
High
Unassigned

Bug Description

The quick_maketext()function in HyperArch.py uses a
caching scheme for templates extracted from one of the
four possible sources of templates, i.e. general, site,
domain and list specific templates.

The caching scheme is broken because the key to the
cache only uses the name of the template file and the
language. Thus once a a file for a given langauge has
been cached, regardless of which of the sources it was
obtained from, it is that template that is used by all lists,
regardless of the existence of a template in a preferred
source for that particular list.

The attached patch fixes this problem.

Apply the patch with the MM build directory as current
working directory using the command:

patch -p1 < path-to-patch-file

[http://sourceforge.net/tracker/index.php?func=detail&aid=730769&group_id=103&atid=100103]

Tags: pipermail
Revision history for this message
Ppsys (ppsys) wrote :
Revision history for this message
Ppsys (ppsys) wrote :

And do not forget to mailmanctl restart after applying the
patch!

Revision history for this message
Ppsys (ppsys) wrote :

After doing some furehtr (proper) testing I hope version 2 of
the patch will solve the problem

Revision history for this message
Barry Warsaw (barry) wrote :

Do we need two caches? Isn't the important point here that
the listname be included in the cache key? If so, maybe we
just need one cache that maps (templatefile, lang, listname)
to the template text.

If you agree, please rework the patch accordingly. Also,
rename Utils.real_maketext() to Utils.findtext() and change
call sites accordingly.

Revision history for this message
Ppsys (ppsys) wrote :

My first reaction to fixing the problem was as bwarsaw
suggests, just add the list name to the cache key.

On consideration I decided the use of a two level cache is a
justified and trivial complication.

The top level cache maps (filename, lang, listname) ->
filepath, potentially a many -> one mapping.

The second level cache maps filepath -> filecontent, always a
one -> one mapping.

A single level cache would map (filename, lang, listname) ->
filecontent, always a one -> one mapping, which would lead
to multiple copies of the same template file content data
being held in memory when multiple lists use the same
template; arguably the norm on Mailman installations. Take
my case of 460 lists using the same default English language
templates.

This waste of storage is trivially avoidable with the second
cache dictionary.

I have updated the patch to tcache-2.1.2-0.3.patch, which
introduces the change in function name from
Utils.real_maketext to Utils.findtext and propagates that
change as necessary.

Footnotes:

The approach adopted is safe as HyperArch.quick_maketext
() always calls Utils.findtext() with dict=None so that
Utils.findtext() does no characterisation of the template
content; this being deferred until HyperArch.quick_maketext()
acts on the cache content.

When multiple lists use the same template file, its contents
will be re-acquired the first time each list first seeks to use it.
This latter is a relatively minor inefficency which I thought
preferable to more radical changes in the original
Utils.maketext function to separate the path determination
and file content acquisition aspects of its behaviour into
different functions.

Revision history for this message
Barry Warsaw (barry) wrote :

Accepted, with minor stylistic modifications. Thanks!

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.