Developers of Sympa

[IKEDA Soji <address@concealed>]

Hi,

On Mon, 10 Mar 2014 19:26:23 +0100
Guillaume Rousse <address@concealed> wrote:

> Le 06/03/2014 15:39, IKEDA Soji a écrit :
> > Developers,
> >
> > I examine not to provide connection pooling and auto reconnect of
> > SDM, but I found it is very costy.  So I restored that feature.
> 'pooling' usually means 'pre-allocating multiple instances of a given 
> resource, to make it immediatly available on demand thereafter'. It 
> makes sense in a web applications context, for instance, where the 
> parent process takes care of creating multiple LDAP or database 
> connection in a *shared* memory section, so as multiple children can 
> subsequently retrieve and use them as needed. Given than the only 
> concurrency context in Sympa (CGI or bulk daemon) relies on isolated 
> processes, unable to share anything, I don't really see any significant 
> interest for pooling the single database connection in use...

My wording was incorrect.  I'll reword that "sustainable connection
and statement caching".

> The only current benefit of this 'pooling' feature is to cache  the 
> database connection (the DBI::DBH instance), because it is lazily 
> created as a side effect of any kind of database operation. Any call to 
> any function in Sympa::DatabaseManager function, such as do_query(), 
> will silently intialize this connection, relying on this caching 
> mechanism to avoid creating duplicate DBI::DBH instances... I could 
> summarize its purpose as "no need to worry where I should store my 
> database handle, as I can throw it away in a loose bag after every 
> usage, and dig this bag again next time I need it". Dubious feature...

I have once explained that using db handle directly will make Sympa
unstable and make code complex.  Please read past posts.

> A far simpler, readable and efficient solution would be to have each 
> binary (sympa.pl, bulk.pl, wwwsympa.fcgi) explicitely create this 
> database connection on startup, check if it is uptodate (otherwise, exit 
> immediately), and keep a permanent reference on it, for later usage. Ie, 
> something as:
> 
> # read configuration
> 
> our $database = Sympa::Database->new(%params);
> croak 'unable to connect to the database, check your configuration'
>      if !$database->connect();
> croak 'database need to be updated first, use sympa_wizard'
>      if !$database->up_to_date();
> 
> # let's assume than the database is now available and fonctional
> # from this point everywhere in the code as $main::database
> 
> 
> 90% of code in Sympa::DatabaseManager would become immediatly useless, a 
> they just forward function call to the underlying $db_source object.

I agree to you on style, and on the fact SDM and Datasource are
redundant.  However, I suppose it would be better rather to unify
the feature above into Datasource than to strip it (I'll write on
this issue later again, about use of singleton).


BTW: The naming "Sympa::DatabaseManager" feels too long for me.
March of such as many "Sympa::DatabaseManager::quote(...)" can be
hard to read.  I prefer to "Sympa::DBM", at least "Sympa::DBManager".


Regards,

--- Soji

-- 
株式会社 コンバージョン  セキュリティ&OSSソリューション部   池田荘児
〒231-0004　神奈川県横浜市中区元浜町3-21-2 ヘリオス関内ビル7F
e-mail address@concealed  TEL 045-640-3550
http://www.conversion.co.jp/