[Catalyst] Session duplicate key constraints on concurrent requests

[Janne Snabb]

On Fri, 7 Oct 2011, Tobias Kremer wrote:

> I appreciate your taking the time to look into this. Unfortunately
> your patches are for the Store::DBI backend, whereas I'm using
> Store::DBIC (DBIx::Class).

So, the code you are talking about is then
Catalyst/Plugin/Session/Store/DBIC/Delegate.pm? It currently has
the following in _load_row():

my $load_sub = sub {
    return $self->model->find_or_create({ $self->id_field => $key })
};

my $row;
if (blessed $self->model and $self->model->can('result_source')) {
    $row = $self->model->result_source->schema->txn_do($load_sub);
}

The code is clearly incorrect. The person writing it probably just
thought "oh well, I'll wrap it in transaction -- it probably helps
here". Even if you have the highest transaction isolation level
"serializable", you will not lock on SELECTs which return no rows.

It is trivial to fix in some ways that I mentioned:

#1. Drop the unique constraint on id_field and add selector_field
which is auto_increment in the table (see my earlier post for more
details) and remove the unneeded transaction:

my $row;
if (blessed $self->model and $self->model->can('result_source')) {
    $row = $self->model->find({ $self->id_field => $key },
                              { order_by => { '-asc' => $self->selector_field },
                                rows => 1 })
        or $self->model->create({ $self->id_field => $key });
}

Note that this requires support for auto_increment/serial type in
the underlying DB. This is the only somewhat generic solution which
is safe with MySQL multi-master asynchronous replication (where you
might be writing to any of several replicants). That is why this
is my generic recommendation.

#2. Implement find_or_create_atomic() in DBIC:

my $row;
if (blessed $self->model and $self->model->can('result_source')) {
    $row = $self->model->find_or_create_atomic({ $self->id_field => $key });
}

#3. Implement "LOCK TABLES" in DBIC and use locks to protect your
critical section:

my $row;
if (blessed $self->model and $self->model->can('result_source')) {
    $self->model->rwlock();
    $row = $self->model->find_or_create({ $self->id_field => $key });
    $self->model->unlock();
}

#4. Catch the duplicate key exception in application code:

my $row;
if (blessed $self->model and $self->model->can('result_source')) {
    $row = $self->model->find({ $self->id_field => $key })
        or eval {
            $self->model->create({ $self->id_field => $key });
        } or 
            $self->model->find({ $self->id_field => $key });
}

(Add the code to get the sqlstate from the DBI driver and check if
the error was integrity constraint violation ($sqlstate =~ /^23/)
if desired.)


Hope this helps,
--
Janne Snabb / EPIPE Communications
snabb at epipe.com - http://epipe.com/