[Mailman-Developers] Analysis of MM2.x Admin slowness with, (not "because of") MySQL adaptor(s).

[Kyrian (List)]

Folks,

Okay, I've actually picked through the code (both the MySQL adaptor and 
Fil's code) on this one and made a couple of observations, and I think 
I've definitively found the culprit here.

We're looking at Mailman/Cgi/admin.py here. Lines 871 onwards, for 
between 5 and 20 lines depending on whether you're looking at Fil's 
modified code or the distributed version.

This is the case (although different code surrounds the offending bits) 
up to and including the latest 2.1.11 version, and of course only if I 
read the Python right ;-)

First pass of the code suggested that Fil's code was a bit wide of the 
mark, although it would work for searches.

This is because the patched code only uses his getMembersMatching() 
method if we are not given an empty search string (and the code uses the 
same search method of getMembers() with an empty string as a defined 
search string), so listing all members in order with Fil's code will be 
missing the "names" array, and be fast, but my original will populate 
the "names" array, and be cripplingly slow for large lists.

This, however is not the source of the problem.

Fil's code seems to only call his getMembersMatching() method to 
populate the 'all' array, and disregards the users "names" array, which 
is populated in the normal version. I don't suppose he's bothered about 
people's 'real name' values ;-)

This is the root cause of the problem, because it's doing a foreach-type 
loop through all the members retrieved by getMembers() to get their 
names individually, rather than making a single database call to get 
them all, which would seem to be the cause of the slowness. It is even 
noted by Barry that this bit of code sucks:

# BAW: There's got to be a more efficient way of doing this!
names = [mlist.getMemberName(s) or '' for s in all]

Therefore I would propose a change to admin.py that calls a member 
adaptor's mlist.getAllMemberNames() if it exists/is implemented, 
otherwise defaults to doing the aforementioned instead, which is ok for 
pickles. The obvious caveat is that getAllMemberNames() MUST return 
members in the same order as the getMembers() function or it'll cause 
funny things to happen.

As long as we know what to sling into the SQL 'ORDER BY' that'll be fine 
though. I'd assume ordering by lower case address is the way it happens 
by default??

I suppose you could just move that offending line of code into a 
getAllMemberNames() function in OldStyleMemberships.py too though?

This would allow SQL to do its thing in two shots (once for names, and 
once for addresses) rather than loads of individual user queries (once 
for addresses, loads of times for names), and remove the bottleneck, 
while also minimising the required mangling of core code.

It would also allow backward compatibility, just *very slow* backward 
compatibility by not calling the new method in the adaptor ;-)

Comments?

K.

-- 
Kev Green, aka Kyrian. E: kyrian@ore.org WWW: http://kyrian.ore.org/
Linux/Security Contractor/LAMP Coder/ISP, via http://www.orenet.co.uk/
                  DJ via http://www.hellnoise.co.uk/