[ mailman-Bugs-730769 ] template access hierarchy is broken

[SourceForge.net]

Bugs item #730769, was opened at 2003-05-01 09:24
Message generated for change (Comment added) made by bwarsaw
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100103&aid=730769&group_id=103

Category: Pipermail
Group: 2.1 (stable)
>Status: Closed
>Resolution: Accepted
Priority: 8
Submitted By: Richard Barrett (ppsys)
Assigned to: Nobody/Anonymous (nobody)
Summary: template access hierarchy is broken

Initial Comment:
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


----------------------------------------------------------------------

>Comment By: Barry A. Warsaw (bwarsaw)
Date: 2003-05-12 11:20

Message:
Logged In: YES 
user_id=12800

Accepted, with minor stylistic modifications.  Thanks!


----------------------------------------------------------------------

Comment By: Richard Barrett (ppsys)
Date: 2003-05-12 10:11

Message:
Logged In: YES 
user_id=75166

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.


----------------------------------------------------------------------

Comment By: Barry A. Warsaw (bwarsaw)
Date: 2003-05-12 00:08

Message:
Logged In: YES 
user_id=12800

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. 


----------------------------------------------------------------------

Comment By: Richard Barrett (ppsys)
Date: 2003-05-01 11:09

Message:
Logged In: YES 
user_id=75166

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

----------------------------------------------------------------------

Comment By: Richard Barrett (ppsys)
Date: 2003-05-01 10:00

Message:
Logged In: YES 
user_id=75166

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

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=100103&aid=730769&group_id=103