[Catalyst-dev] Bug in Catalyst::Model::DBI .15

Alex Pavlovic alex at taskforce-1.com
Sun May 6 20:38:31 GMT 2007


Hi,

On Sunday 06 May 2007 09:37, Evan Carroll wrote:
> > Let's start off with a brief description of the methods of M::DBI.
> > Currently, there is no connection to the database on ->new(). The
> > connection is established on the first call to ->dbh, when M::DBI
> > realizes it isn't connected, and proceeds to connect. This appears to
> > work, though it's really just a side-effect of a method
> > ->stay_connected, being called prior to an official ->connect. 

It's not a side effect, it's called deferred initiliazation.

> > This 
> > works as long as $c->model('x') refers to the same instance, and NOT a
> > rebless of that compile-time instance.

Every model in catalyst that you have is a new instance, therefore it results 
in new connection, if this is what you meant. It is obvious re-connect will 
be made, because $dbh is stored with each original instance. 

This is precisely why I wrote the proxy class, where operations are just being 
forwarded to one centralized instance that has them cached and you need none 
of the ACCEPT_CONTEXT magic.

> >
> > The problem is with ACCEPT_CONTEXT and the current version of
> > Catalyst::Model::DBI.
> >

While this is a problem, you have to realize that this is still only a problem 
for people who use the model in such way that you do. I will work towards a 
patch so that it may satisfy your needs. If you are inclined or have already 
started any work, please inform me directly.

> > You have to have a connection to the database, prior to having ->dbh
> > called. The most common use of ACCEPT_CONEXT looks like this:
> >
> > sub ACCEPT_CONTEXT {
> >  my ( $self, $c ) = @_;
> >  bless { %$self, c => $c}, ref ( $self );
> > }
> >
> > What this does is simple, it gives your model access to $c, and
> > ensures it's unique per request.
> >
> > In this example, on each request, the object of an unconnected model,
> > is reblessed into a new object, where it establishes connection upon
> > the first call to ->dbh, unique from the compile-time copy of M::DBI.
> > In other words, on each call to $c->model('DBI')->dbh, a new
> > unconnected instance will have to connect, and because it's a rebless,
> > its connection won't be cached ( it will neither be the same for
> > another $c->model, nor will it exist outside of the one response. )
> >
> > So if you call this twice in your Controller:
> > die $c->model('DBI')->dbh->{pg_pid}, $c->model('DBI')->dbh->{pg_pid}
> > OR if you use the power this provides to call another model, that
> > wraps around an M::DBI class, ex:
> >
> > package M::DBI::FooTable;
> > sub create {
> >        my $self = shift;
> >        my $c = $self->{c};
> >        my $fkey_bar = $c->model('DBI::BarTable')->create(@_); ## this
> > model communicates on a different db session
> > }
> >
> > You get failure.
> >
> > I need this functionality because I use a different model for each
> > table, and on close inspection the reconnecting bit is a red flag. But
> > with the current implementation, you simply can not have cross table
> > transactions using the model-per-table method I've chosen.

You can, but not the way you are doing it at the moment.

> >
> > With that said, the patch provided has no drawbacks, and even if you
> > only use the most minimal functionality of M::DBI, with the current
> > code, you don't have a connection until the first ->dbh is called,
> > which is no bueno. (work that is better handled at compile-time)
> >
> > The grief was simply the nature of the problem, and having to
> > understand so much about the way the technology works to find the bug,
> > which isn't evident in most use cases.

Please understand that this is community driven project. Nothing is perfect. 
Lot of us here dedicate our free time to work on these things.

> >
> > On 5/5/07, Alex Pavlovic <alex at taskforce-1.com> wrote:
> > > Hi,
> > >
> > > On Friday 04 May 2007 19:01, Evan Carroll wrote:
> > > > M::DBI has a pretty sizable bug in as is such that it will establish
> > > > a new DBI handle on every incoming connection.
> > >
> > > In what environment CGI, FCGI, mod_perl ? You did not mention.
> > >
> > > > This is a problem for
> > > > speed, and transactions as it makes it much more difficult to share
> > > > $dbh across other models,
> > >
> > > If you want to share $dbh across multiple models then take a look at:
> > >
> > > http://search.cpan.org/author/ALEXP/Catalyst-Model-Proxy-0.03/lib/Catal
> > >yst/Model/Proxy.pm
> > >
> > > It has already been done.
> > >
> > > > or to wrap them. I'm very surprised this
> > > > went for such a long time undiscovered.
> > >
> > > What went on undiscovered ? I still dont understand what you are trying
> > > to accomplish. You can't share dbh across threads or multiple
> > > processes( because each connection results in distinct namespace ).
> > > DBIx::Class::Storage does the same thing, look at:
> > >
> > > http://search.cpan.org/~blblack/DBIx-Class-0.07006/lib/DBIx/Class/Stora
> > >ge/DBI.pm
> > >
> > > <code>
> > > elsif($self->_conn_pid != $$) {
> > >           $self->_dbh->{InactiveDestroy} = 1;
> > >           return $self->_dbh(undef);
> > > }
> > > </code>
> > >
> > > > This one line caused a lot of
> > > > grief.
> > >
> > > What kind of grief ?
> > >
> > > > Thanks goes to >nothingmuch for all his help in first blaming the
> > > > problem totally on my style, and then later totally on M::DBI.
> > >
> > > I assume this was on irc. Next time please contact me before posting
> > > anything, it will save you some time and "grief". Not sure what your
> > > patch does here, maybe you can clarify.
> > >
> > > > root at x60s:/usr/local/share/perl/5.8.8/Catalyst/Model# diff DBI.pm
> > > > DBI_FIX.pm 59a60,62
> > > >
> > > > >       $self->_dbh( $self->connect );
> > > >
> > > > 72c75
> > > > <                       } elsif ( $self->_pid != $$ ) {
> > > > ---
> > > >
> > > > >       } elsif ( $self->_pid != $$ ) {
> > > >
> > > > --
> > > > Evan Carroll
> > > > System Lord of the Internets
> > > > me at evancarroll.com
> > > > 832-445-8877
> > >
> > > --
> > > Alex Pavlovic - CTO
> > > TF-1 Inc. ( Custom development, consultancy and training )
> > > http://taskforce-1.com
> >
> > --
> > Evan Carroll
> > System Lord of the Internets
> > me at evancarroll.com
> > 832-445-8877
> >
> >
> >
> > ------------------------------
> >
> > Message: 2
> > Date: Sun, 6 May 2007 11:29:48 +0200
> > From: "A. Pagaltzis" <pagaltzis at gmx.de>
> > Subject: [Catalyst-dev] Re: Bug in Catalyst::Model::DBI .15
> > To: catalyst at lists.rawmode.org, catalyst-dev at lists.rawmode.org
> > Message-ID: <20070506092948.GN20785 at klangraum>
> > Content-Type: text/plain; charset=utf-8
> >
> > * Evan Carroll <lists at evancarroll.com> [2007-05-06 10:55]:
> > > With that said, the patch provided has no drawbacks, and even
> > > if you only use the most minimal functionality of M::DBI, with
> > > the current code, you don't have a connection until the first
> > > ->dbh is called, which is no bueno. (work that is better
> > > handled at compile-time)
> >
> > That is precisely the patch's drawback. Connecting lazily rather
> > than eagerly is a feature.
> >
> > However, that's not hard to correct, although the patch would be
> > larger: rather than squirreling away a connection in `new`,
> > squirrel away a reference to an undef scalar. Then you can assign
> > the handle to that scalar, and it will be shared among everyone
> > who kept a copy of the same reference.
>
> I don't see eye to eye with you on this one, knowing if you fail to
> connect to a DB prior to pulling up a page that requires it seems like
> a good functional bonus. On the flip side, I'm less concerned with
> this minor detail than the larger issue of getting it fixed.
>
> > --
> > *AUTOLOAD=*_;sub _{s/(.*)::(.*)/print$2,(",$\/"," ")[defined
> > wantarray]/e;$1} &Just->another->Perl->hack;
> > #Aristotle
> >
> >
> >
> > ------------------------------
> >
> > _______________________________________________
> > Catalyst-dev mailing list
> > Catalyst-dev at lists.rawmode.org
> > http://lists.rawmode.org/mailman/listinfo/catalyst-dev
> >
> >
> > End of Catalyst-dev Digest, Vol 23, Issue 2
> > *******************************************

-- 
Alex Pavlovic - CTO
TF-1 Inc. ( Custom development, consultancy and training )
http://taskforce-1.com



More information about the Catalyst-dev mailing list