[Dbix-class] DBIx::Class::Ordered changes.

[Bill Moseley]

On Wed, Sep 19, 2012 at 3:34 AM, Peter Rabbitson <rabbit+dbic at rabbit.us>wro=
te:

> > UPDATE ordered_list SET position =3D position - 1 WHERE ( ( ( position
> > BETWEEN ? AND ? ) AND owner =3D ? ) ): '13', '18', '1163299'
> >
> > The problem, of course, is I have a UNIQUE( position, owner ) and that
> last
> > update isn't guaranteed to work in order (from 13 to18 -- which would
> > prevent duplicate key errors).
>
> The assumption was that databases are able to handle this correctly. Plea=
se
> revert the patch[1] removing this functionality (as per its commit messag=
e)
> and let us know if this solves your problem. Also I'd recommend filing an
> RT
> bugreport to keep track of this.
>

I've discussed this Ordered code a bit here on the list.   The problem is
I'm not sure what is the correct fix.  I'm not convinced that the old code
is better.   I like the idea of doing the move in a single query.

I can make the UNIQUE( position, group ) constraint DEFERRABLE INITIALLY
DEFERRED but there's still race conditions. There's room for other updates
to sneak in between the select and update.   It's the race conditions that
have seemed to been the most problem in production.    As much as I avoid
locking, seems like locking is what is needed.


Years back I added this code for inserts -- where historically we were
seeing the most duplicate key errors.   After discussion on the Postgresql
list I ended up doing a select for update *on the related grouping row*.


sub insert {

    # shifting self off @_ so other args can be passed to
$self->next::method
    my $self =3D shift;


    # If a position value is provided then we just continue without locking
    # as the caller is explicitly setting the number.
    # (Might be good to throw and exception, instead, if wish to enforce.)

    return $self->next::method( @_ )
        if defined $self->get_column( $self->position_column );



    # We must have a grouping column defined to do the locking
    # And Ordered won't throw that helpful of a message.

    my $group_column =3D $self->grouping_column
        || die 'Ordered needs a grouping_column defined in your class: ' .
ref $self;



    my $new_row;

    my $next_insert =3D $self->next::can;

    $self->result_source->schema->txn_do( sub {

            # Lock with the SELECT FOR UPDATE
            $self->select_related_for_update( $group_column );

            # Now do the Ordered insert while locked.
            $new_row =3D $self->$next_insert( @_ );
    } );

    return $new_row;
}


sub select_related_for_update {
    my ( $self, $grouping_column ) =3D @_;

    my ( $table, $pk ) =3D $self->find_table_for_related_column(
$grouping_column );

    # The dbh method selects the master, not the replicants.
    my $dbh =3D $self->result_source->storage->dbh;

    my $sth =3D $dbh->prepare( "SELECT 1 FROM $table WHERE $pk =3D ? FOR
UPDATE" );

    $sth->execute( $self->get_column( $self->grouping_column ) );
    $sth->finish;

    return;
}








>
> Cheers
>
> [1]
> http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=3Ddbsrgits/DBIx-Class.git;=
a=3Dcommitdiff;h=3D5e6fde33e5a4
>
> _______________________________________________
> List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class
> IRC: irc.perl.org#dbix-class
> SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/
> Searchable Archive:
> http://www.grokbase.com/group/dbix-class@lists.scsys.co.uk
>



-- =

Bill Moseley
moseley at hank.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.scsys.co.uk/pipermail/dbix-class/attachments/20120919/d40=
30183/attachment.htm