Skip Menu |
 

This queue is for tickets about the DBIx-Class CPAN distribution.

Report information
The Basics
Id: 129816
Status: open
Priority: 0/
Queue: DBIx-Class

People
Owner: Nobody in particular
Requestors: ether [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



Subject: BlockRunner not handling rollback-during-commit gracefully
Download (untitled) / with headers
text/plain 3.3k
(#dbix-class, 2019-06-13) Hopefully this is enough information. Because the issue involves interacting with a real db driver, I don't think it's possible to easily write this up as a unit test. 10:22 < ether> bah, it looks like deferred constraint checking is not well-handled in DBD::Pg... 10:22 < ether> I've got a bunch of statements wrapped in a transaction. some of the constraints deferred until that 'COMMIT' is executed... 10:23 < ether> I'm using txn_do, so a ROLLBACK is then triggered 10:23 < ether> and then I get a warning: rollback ineffective with AutoCommit enabled at /Users/ether/.perlbrew/libs/26.3@std/lib/perl5/DBIx/Class/Storage/DBI.pm line 1651. 10:24 < ether> which is a red herring in this case, because everything did actually get successfully rolled back 10:24 < ether> I can't find this warning string in .pm code so it must be in XS somewhere, in the driver itself 10:25 < ether> all I can think to do is to set up a temporary __WARN__ handler to swallow this warning, where I set up the txn_do 10:26 < ether> ah, it's in Driver.xst in the DBI distribution 10:27 < ether> https://metacpan.org/source/TIMB/DBI-1.642/Driver.xst#L305-312 10:28 < ether> I'm not quite sure how one could patch this for this situation, though 10:29 < ether> perhaps I could temporarily turn off AutoCommit on the dbh 10:29 < ether> or should DBIx::Class::Storage::txn_rollback be doing this? 10:30 < alh> Why is autocommit on if you're in a tx? 10:31 < ether> I don't think it get turned off for transactions 10:34 < mohawk> i thought it did 10:34 < ether> I don't have this problem for normal rollbacks inside transactions -- this one is special because we already issued a 'COMMIT' and the exception (triggering a rollback) happened during that execution 10:34 < ether> (because of the deferred constraints) 10:35 < mohawk> sounds like DBI itself might be turning autocommit back on a bit prematurely to me 10:35 <@mst> oh, that's interesting 10:35 <@mst> mohawk: naw 10:36 < ether> ? 10:36 <@mst> this'll be "DBIC tries to rollback on exception, it hadn't occurred to DBIC that commit could be what's throwing it" 10:36 <@mst> I'd bet 10:36 <@mst> which means it's not really DBD::Pg's fault 10:37 < ether> if it's DBIx::Class's fault, that means I should be able to patch around it 10:37 < ether> adding a __WARN__ handler is hacky but perhaps safest 10:38 <@mst> BlockRunner.pm seems to be where it's getting confused 10:40 < mohawk> mst, does that mean DBIC itself is changing AutoCommit? 10:40 <@mst> what 10:41 <@mst> of course it is, that's how transactions *work* 10:41 < ether> I thought the 'start transaction' statement might override AutoCommit 10:41 < mohawk> the last part i am aware of. my mental model was that DBI (not DBIC) took care of all of that, and DBIC just called txn_do (which i will now confirm is a DBI thing by looking that up) 10:42 <@mst> no, txn_do I invented 10:42 <@mst> however, calling $dbh->begin_work will IIRC set AutoCommit to 0 10:43 < mohawk> mst, thanks - that was where my model was wrong 10:44 < hobbs> txn_do appeared in DBIC and then was extracted into a couple modules which can provide the same thing on top of DBI without the rest of DBIC 10:44 <@mst> right 10:44 <@mst> because once I'd implemented it, it was obviously superior, so in the true spirit of open source people stole it enough times that it just started to seem obvious to people
Subject: Re: [rt.cpan.org #129816] BlockRunner not handling rollback-during-commit gracefully
Date: Sat, 15 Jun 2019 11:17:15 +0200
To: bug-DBIx-Class [...] rt.cpan.org
From: Peter Rabbitson <ribasushi [...] leporine.io>
Download (untitled) / with headers
text/plain 540b
On 06/13/2019 07:45 PM, Karen Etheridge via RT wrote: Show quoted text
> > Hopefully this is enough information.
I am unable to glean much insight from the attached log. Show quoted text
> Because the issue involves interacting with a real db driver, I don't think it's possible to easily write this up as a unit test.
This assumption is incorrect. Please augment the currently passing test to mimic your failure-case: https://github.com/Perl5/DBIx-Class/blob/maint/0.0828xx/t/72pg.t#L442-L490 A fix would be rather obvious once your failure scenario is understood.
Download (untitled) / with headers
text/plain 289b
On Sat Jun 15 11:22:48 2019, RIBASUSHI wrote: Show quoted text
> Please augment the currently passing > test > to mimic your failure-case: > https://github.com/Perl5/DBIx-Class/blob/maint/0.0828xx/t/72pg.t#L442- > L490 > > A fix would be rather obvious once your failure scenario is > understood.
Ping?
Download (untitled) / with headers
text/plain 846b
On 2019-07-23 01:27:17, RIBASUSHI wrote: Show quoted text
> On Sat Jun 15 11:22:48 2019, RIBASUSHI wrote: >
> > Please augment the currently passing > > test > > to mimic your failure-case: > > https://github.com/Perl5/DBIx-Class/blob/maint/0.0828xx/t/72pg.t#L442- > > L490 > > > > A fix would be rather obvious once your failure scenario is > > understood.
> > > Ping?
See attached patch. With DBIC_TRACE=1, it creates the following output: BEGIN WORK # starting transaction INSERT INTO artist ( name, rank) VALUES ( ?, ? ) RETURNING artistid: 'bob', '1' INSERT INTO artist ( name, rank) VALUES ( ?, ? ) RETURNING artistid: 'bob', '2' COMMIT ROLLBACK rollback ineffective with AutoCommit enabled at lib/DBIx/Class/Storage/DBI.pm line 1651. not ok 511 - got no exception on rollback # Failed test 'got no exception on rollback' # at t/72pg.t line 518.
Subject: 0001-repro-case-for-RT-129816.patch
From d19577cf257764d507bf778847b9728f34ee5308 Mon Sep 17 00:00:00 2001 From: Karen Etheridge <ether@cpan.org> Date: Tue, 23 Jul 2019 14:40:47 -0700 Subject: [PATCH] repro case for RT#129816 --- t/72pg.t | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/t/72pg.t b/t/72pg.t index e9c5ce16..d7fa2e5c 100644 --- a/t/72pg.t +++ b/t/72pg.t @@ -489,6 +489,35 @@ lives_ok { $cds->update({ year => '2010' }) } 'Update on prefetched rs'; } [], 'No warnings on deferred rollback'; } +# repro case for https://rt.cpan.org/Ticket/Display.html?id=129816 +{ + my $schema = DBICTest::Schema->connect($dsn, $user, $pass); + drop_test_schema($schema); + create_test_schema($schema); + use Try::Tiny; + my $exception; + try { + $schema->txn_do(sub { + print STDERR "# starting transaction\n"; + $schema->storage->dbh_do(sub { $_[1]->do('set constraints all deferred') }); + + # now do something that will cause a constraint violation *at commit time* + $schema->resultset('Artist')->create({ + name => 'bob', rank => 1, + }); + $schema->resultset('Artist')->create({ + name => 'bob', rank => 2, + }); + + }); + } + catch { + $exception = $_; + }; + + ok(!$exception, 'got no exception on rollback'); +} + done_testing; END { @@ -511,7 +540,7 @@ sub create_test_schema { my $std_artist_table = <<EOS; ( artistid serial PRIMARY KEY - , name VARCHAR(100) + , name VARCHAR(100) unique deferrable , rank INTEGER NOT NULL DEFAULT '13' , charfield CHAR(10) , arrayfield INTEGER[] -- 2.22.0
Download (untitled) / with headers
text/plain 247b
Sorry, the test on $exception there is wrong -- of course we expect an exception, we just don't expect a warning. Nevertheless, it reproduces the issue I was describing -- a warning is generated on commit->die->rollback that should be suppressed.
Download (untitled) / with headers
text/plain 434b
Better patch. Output: ... BEGIN WORK INSERT INTO artist ( name, rank) VALUES ( ?, ? ) RETURNING artistid: 'bob', '1' INSERT INTO artist ( name, rank) VALUES ( ?, ? ) RETURNING artistid: 'bob', '2' COMMIT ROLLBACK not ok 511 - got no warnings on rollback # Failed test 'got no warnings on rollback' # at t/72pg.t line 513. # got warnings: rollback ineffective with AutoCommit enabled at lib/DBIx/Class/Storage/DBI.pm line 1651.
Subject: 0001-repro-case-for-RT-129816.patch
From ac27d3e4c1950316fbb1c061a9589d5c5f193160 Mon Sep 17 00:00:00 2001 From: Karen Etheridge <ether@cpan.org> Date: Tue, 23 Jul 2019 14:40:47 -0700 Subject: [PATCH] repro case for RT#129816 --- t/72pg.t | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/t/72pg.t b/t/72pg.t index e9c5ce16..a1570a59 100644 --- a/t/72pg.t +++ b/t/72pg.t @@ -489,6 +489,31 @@ lives_ok { $cds->update({ year => '2010' }) } 'Update on prefetched rs'; } [], 'No warnings on deferred rollback'; } +# repro case for https://rt.cpan.org/Ticket/Display.html?id=129816 +{ + my $schema = DBICTest::Schema->connect($dsn, $user, $pass); + drop_test_schema($schema); + create_test_schema($schema); + my @warnings; + local $SIG{__WARN__} = sub { push @warnings, $_[0] }; + eval { + $schema->txn_do(sub { + $schema->storage->dbh_do(sub { $_[1]->do('set constraints all deferred') }); + + # now do something that will cause a constraint violation *at commit time* + $schema->resultset('Artist')->create({ + name => 'bob', rank => 1, + }); + $schema->resultset('Artist')->create({ + name => 'bob', rank => 2, + }); + }); + }; + + ok(!@warnings, 'got no warnings on rollback') + or diag 'got warnings: ', explain @warnings; +} + done_testing; END { @@ -511,7 +536,7 @@ sub create_test_schema { my $std_artist_table = <<EOS; ( artistid serial PRIMARY KEY - , name VARCHAR(100) + , name VARCHAR(100) unique deferrable , rank INTEGER NOT NULL DEFAULT '13' , charfield CHAR(10) , arrayfield INTEGER[] -- 2.22.0


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.