[Rt-devel] Postgres performance issues and some "odd" ways to get IDs...

[Joby Walker]

Jesse,

The new behavior seems very broken:

1) Insert row (sequence nextval returns x)
2) get current value of sequence (returns x)

As far as I can see SearchBuilder doesn't wrap a transaction around the 
Insert sub so you could end up with unpredictable results:

1) Process 1: Insert row (sequence nextval returns x)
2) Process 2: Insert row (sequence nextval returns x+1)
3) Process 1: get current value of sequence (x+1)
4) Process 2: get current value of sequence (x+1)

Wrapping a transaction around it should solve the problem, but you would 
be adding transaction overhead for no gain.  What Matt wrote is the best 
behavior (it is also what other DB object models use such as PHP's Propel):

1) get nextval of sequence (x)
2) insert row with id of x.

This cannot be munged by multiple processes:

1) Process 1: get nextval of sequence (x)
2) Process 2: get nextval of sequence (x+1)
3) Process 1: insert row (id = x)
4) Process 2: insert row (id = x+1)

While it is unlikely that the SearchBuilder behavior would actually end 
up with a wrong result, the probability is non-zero.  Very little code 
change would be necessary (see attached -- and I haven't tested 
this...).  The only downside would be a gap in the sequence when the 
insert failed.

Joby Walker
C&C SSG, University of Washington


Jesse Vincent wrote:
> 
> 
> On Tue, Aug 15, 2006 at 01:24:30AM +1000, Matthew Sullivan wrote:
>> Hey All,
>>
>> the only slow queries... needless to say I was surprised when I saw what 
>> it was doing (eg: SELECT id from Transactions where oid = <blah>;) ... 
>> further inspection lead me to DBIx::SearchBuilder::Handle::Pg::Insert() 
>> ... and the following:
> 
> Why don't you try upgrading DBIx::SearchBuilder:
> 
> http://search.cpan.org/src/JESSE/DBIx-SearchBuilder-1.43/SearchBuilder/Handle/Pg.pm
>> sub Insert {
> ...
>>    my $sql = "SELECT id FROM $table WHERE oid = ?";
>>    my @row = $self->FetchResult($sql, $oid);
>>    # TODO: Propagate Class::ReturnValue up here.
>>
>> Comments/flames welcome...
>>
>> Regards,
>>
>> Mat
>>
>> PS: It's 1:15am here after 6 hours of database performance debugging, 
>> apologies if the above offends, it is not meant to.
>>
> 
> But maybe only after you get a bit of rest. Best luck. Please tell us if
> this helps.
> 
> Jesse
> _______________________________________________
> List info: http://lists.bestpractical.com/cgi-bin/mailman/listinfo/rt-devel
-------------- next part --------------
--- Pg.pm	2006-08-14 10:00:28.255968250 -0700
+++ Pg.pm.new	2006-08-14 10:01:07.138398250 -0700
@@ -61,24 +61,23 @@
     my $self  = shift;
     my $table = shift;
     my %args  = (@_);
+
+    unless($args{'id} || $args{'Id'}) {
+        my $sequence_name = $self->IdSequenceName($table);
+        unless ($sequence_name) { return ($sequence_name) }   
+        my $seqsth = $self->dbh->prepare(
+            qq{SELECT NEXTVAL('} . $sequence_name . qq{')} );
+        $seqsth->execute;
+        $args->{'id'} = $seqsth->fetchrow_array();
+    }
+
     my $sth   = $self->SUPER::Insert( $table, %args );
 
     unless ($sth) {
         return ($sth);
     }
 
-    if ( $args{'id'} || $args{'Id'} ) {
-        $self->{'id'} = $args{'id'} || $args{'Id'};
-        return ( $self->{'id'} );
-    }
-
-    my $sequence_name = $self->IdSequenceName($table);
-    unless ($sequence_name) { return ($sequence_name) }   # Class::ReturnValue
-    my $seqsth = $self->dbh->prepare(
-        qq{SELECT CURRVAL('} . $sequence_name . qq{')} );
-    $seqsth->execute;
-    $self->{'id'} = $seqsth->fetchrow_array();
-
+    $self->{'id'} = $args{'id'} || $args{'Id'};
     return ( $self->{'id'} );
 }