Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: ModPerl: ModPerl

Apache::DBI patches

 

 

ModPerl modperl RSS feed   Index | Next | Previous | View Threaded


joet at tellme

Sep 2, 2004, 12:19 PM

Post #1 of 10 (4153 views)
Permalink
Apache::DBI patches

A couple days ago, I posted a patch that fixed what I think is a real bug in
Apache::DBI when a script uses more than one database handle:

http://mathforum.org/epigone/modperl/stroblangnerm

Basically, I removed $Idx as a file-scoped variable, because if a script
used more than one database handle, the initial value of $Idx would get
overwritten, and the first database handle would never get to the
cleanup/rollback routine.

Back in July 2003, Patrick Mulvany posted a more substantial patch that
ensures that database handles get restored to their expected state if, for
example, a script changes AutoCommit, and doesn't change it back:

http://mathforum.org/epigone/modperl/gripemneh

With Patrick's patch, the next user of the database handle won't see the
effects of the first script's fiddling with the AutoCommit property.

Both patches are working well for me, and I was surprised to see no
discussion of either on the mod_perl list. Patrick's patch needed a small
change to work with mine, so I've taken the liberty of combining them in the
attached file.

I'm also cc'ing Ask, as I think this should be included in Apache::DBI. Of
course, feedback from others is welcome.

Thanks,
Joe
Attachments: combinedpatch (7.26 KB)


pgollucci at p6m7g8

Aug 9, 2005, 12:14 PM

Post #2 of 10 (3978 views)
Permalink
Re: Apache::DBI patches [In reply to]

Joe Thomas wrote:
> Hi Philip,
>
> I notice that you're maintaining a new version of Apache::DBI. About a
> year ago, I posted a couple of patches to the mod_perl list, and sent
> them to Ask, but never received any response. My patch fixes a problem
> with database rollback/cleanup when more than one database handle is
> used. It fit well with another (similarly ignored) patch posted by
> Patrick Mulvany in 2003. His patch caches many of the database state
> attributes (AutoCommit, PrintError, RaiseError, etc.), and restores them
> to their initial values when a handle is reused.
>
> The attached patch applies to Apache-DBI-0.99 from your site, and
> includes both changes. Can you take a look and let me know if you'd
> consider including them?

I'll attempt take a look at this sometime this week. At first glance I
think I have some questions about it. I'll get back to you after more
thorough investigation.

Please CC the modperl [at] perl list as I'm not technically the
maintainer, Ask still is, and its supposed to a community maintained module.

Thanks for the work and repeated effort.

P.S.
I CC'ed the list.
--
END
---------------------------------------------------------
What doesn't kill us can only make us stronger.
Nothing is impossible.

Philip M. Gollucci (pgollucci [at] p6m7g8) 301.254.5198
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Developer / Liquidity Services, Inc.
http://www.liquidityservicesinc.com
http://www.liquidation.com
http://www.uksurplus.com
http://www.govliquidation.com
http://www.gowholesale.com
Attachments: 0.99.patch (7.88 KB)


joet at tellme

Aug 9, 2005, 1:26 PM

Post #3 of 10 (3943 views)
Permalink
RE: Apache::DBI patches [In reply to]

> I'll attempt take a look at this sometime this week. At first glance
I
> think I have some questions about it. I'll get back to you after more

> thorough investigation.
>
> Please CC the modperl [at] perl list as I'm not technically the
> maintainer, Ask still is, and its supposed to a community maintained
module.
>
> Thanks for the work and repeated effort.
>
> P.S.
> I CC'ed the list.
>

Thanks, Philip. I think I mangled the patch a bit around line 110
("if(!$Rollback{$Idx} and $needCleanup and
Apache->can('push_handlers'))"). I'll post a fixed version to the list.

Joe


pgollucci at p6m7g8

Aug 9, 2005, 1:29 PM

Post #4 of 10 (3952 views)
Permalink
Re: Apache::DBI patches [In reply to]

Joe Thomas wrote:
>> I'll attempt take a look at this sometime this week. At first glance
> I
>> think I have some questions about it. I'll get back to you after more
>
>> thorough investigation.
>>
>> Please CC the modperl [at] perl list as I'm not technically the
>> maintainer, Ask still is, and its supposed to a community maintained
> module.
>> Thanks for the work and repeated effort.
>>
>> P.S.
>> I CC'ed the list.
>>
>
> Thanks, Philip. I think I mangled the patch a bit around line 110
> ("if(!$Rollback{$Idx} and $needCleanup and
> Apache->can('push_handlers'))"). I'll post a fixed version to the list.

While you're at it, can you post 2 separate ones instead of 1.

Thanks.

--
END
---------------------------------------------------------
What doesn't kill us can only make us stronger.
Nothing is impossible.

Philip M. Gollucci (pgollucci [at] p6m7g8) 301.254.5198
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Developer / Liquidity Services, Inc.
http://www.liquidityservicesinc.com
http://www.liquidation.com
http://www.uksurplus.com
http://www.govliquidation.com
http://www.gowholesale.com


joet at tellme

Aug 9, 2005, 2:32 PM

Post #5 of 10 (3983 views)
Permalink
RE: Apache::DBI patches [In reply to]

> While you're at it, can you post 2 separate ones instead of 1.

Here's the first patch. It moves $Idx from being a file-scoped variable
to connect() scope. The cleanup handler is then passed a closure
containing $Idx:

$s->push_handlers("PerlCleanupHandler", sub { cleanup($Idx) });

or

Apache->push_handlers("PerlCleanupHandler", sub { cleanup($Idx)
});

This makes the cleanup handler work correctly with scripts that use more
than one database handle. Without it, the second DBI->connect call will
overwrite the initial value of $Idx, so the first handle will never get
properly cleaned up.

Joe
Attachments: 0.99-joet.patch (2.73 KB)


joet at tellme

Aug 9, 2005, 3:18 PM

Post #6 of 10 (3974 views)
Permalink
RE: Apache::DBI patches [In reply to]

These patches were originally by Patrick Mulvany, and posted to mod_perl
in 2003. I've made sure they apply to Apache-DBI-0.99, and are
compatible with my re-scoping of $Idx.

The ping patch looks obviously correct to me. It makes sure that the
database is always pinged on connect() if PingTimeOut is set to 0.

Here's the original description of the "reset" patch:

patch from Patrick Mulvany July 21, 2003
- Fixed issues relating to changing handle state post
connection.
Handles now returned in same state as original and incomplete
transactions rolled back before re-issuing handle so
begin_work
should now be safe. "Patrick Mulvany" <paddy [at] firedrake>

Joe
Attachments: 0.99-patrick-reset.patch (4.62 KB)
  0.99-patrick-ping.patch (0.76 KB)


pgollucci at p6m7g8

Aug 9, 2005, 10:12 PM

Post #7 of 10 (3952 views)
Permalink
Re: Apache::DBI patches [In reply to]

Hi,

I agree with all 3 patches.

Thanks for splitting them out. That made my life so much easier.

HTH

--------------
The URL

http://people.apache.org/~pgollucci/CPAN/Apache-DBI/Apache-DBI-0.100.tar.gz

has entered CPAN as

file: $CPAN/authors/id/P/PG/PGOLLUCCI/Apache-DBI-0.100.tar.gz
size: 31562 bytes
md5: dc271d79f7d82508b1232eaac6a1fa53

No action is required on your part
Request entered by: PGOLLUCCI (Philip M. Gollucci)
Request entered on: Wed, 10 Aug 2005 05:07:30 GMT
Request completed: Wed, 10 Aug 2005 05:08:47 GMT

Thanks,

--------------
Changes:
0.100 08/10/2005

- Move $Idx from a file-scoped variable to a connect() scoped
variable, which gets passed to other subroutines as needed.
This will ensure that the cleanup/rollback feature will work
properly when a script uses more than one database handle to the
same database.
[Joe Thomas <joet [at] tellme>]

- Fixed issues relating to changing handle state post
connection. Handles now returned in same state as original and
incomplete transactions rolled back before re-issuing handle so.
Submited by: [Joe Thomas <joet [at] tellme>]
Contributed by: [Patrick Mulvany <paddy [at] firedrake>]

- Fix a () bug in the connect() determining whether we must ping
the database. PingTimeOut = 0 now works as documented.
Submited by: [Joe Thomas <joet [at] tellme>]
Contributed by: [Patrick Mulvany <paddy [at] firedrake>]


--
END
------------------------------------------------------------
What doesn't kill us can only make us stronger.
Nothing is impossible.

Philip M. Gollucci (pgollucci [at] p6m7g8) 301.254.5198
Consultant / http://p6m7g8.net/Resume/
Senior Developer / Liquidity Services, Inc.
http://www.liquidityservicesinc.com
http://www.liquidation.com
http://www.uksurplus.com
http://www.govliquidation.com
http://www.gowholesale.com


autarch at urth

Aug 10, 2005, 1:56 PM

Post #8 of 10 (3954 views)
Permalink
Re: Apache::DBI patches [In reply to]

On Wed, 10 Aug 2005, Philip M. Gollucci wrote:

> http://people.apache.org/~pgollucci/CPAN/Apache-DBI/Apache-DBI-0.100.tar.gz

0.100 < 0.99 !

The CPAN indexer may not like this, and the CPAN shell will certainly not
like it.


-dave

/*===================================================
VegGuide.Org www.BookIRead.com
Your guide to all that's veg. My book blog
===================================================*/


pgollucci at p6m7g8

Aug 10, 2005, 2:00 PM

Post #9 of 10 (3954 views)
Permalink
Re: Apache::DBI patches [In reply to]

Dave Rolsky wrote:
> 0.100 < 0.99 !
>
> The CPAN indexer may not like this, and the CPAN shell will certainly
> not like it.
Its not going to be indexed anyway as I don't have the perms. Ask does.
But, thats a good catch. Whats the next version number after .99 ?
Do I make a 1.00 or a 0.991?


autarch at urth

Aug 10, 2005, 2:26 PM

Post #10 of 10 (3951 views)
Permalink
Re: Apache::DBI patches [In reply to]

On Wed, 10 Aug 2005, Philip M. Gollucci wrote:

> Its not going to be indexed anyway as I don't have the perms. Ask does.
> But, thats a good catch. Whats the next version number after .99 ?
> Do I make a 1.00 or a 0.991?

That's up to you, I guess.

As for perms, Ask can give you co-maintainership via pause.perl.org


-dave

/*===================================================
VegGuide.Org www.BookIRead.com
Your guide to all that's veg. My book blog
===================================================*/

ModPerl modperl RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.