[Dbix-class] Issue with DBIx::Class::ResultSet::find_or_create():

[Mesdaq, Ali]

This came up in the Catalyst list today so I am posting here for
discussion or bug reporting.

The issue is that it seems the find_or_create() call can have a hiccup
in high load situations where find_or_create is called more than once at
the same time with the same value. So what happens is both calls try to
do a find and return nothing then they attempt to insert and once
succeeds and one gets an error of:

DBIx::Class::ResultSet::find_or_create(): DBI Exception:
DBD::mysql::st execute failed: Duplicate entry

The code for find_or_create is

sub find_or_create {
  my $self     = shift;
  my $attrs    = (@_ > 1 && ref $_[$#_] eq 'HASH' ? pop(@_) : {});
  my $hash     = ref $_[0] eq 'HASH' ? shift : {@_};
  my $exists   = $self->find($hash, $attrs);
  return defined $exists ? $exists : $self->create($hash);
}

I started to dig into the source to show where you guys can handle this
but with so much abstraction and 10 modules later I realized its
probably best if I just let you guys decide how to handle it. But we
have had this issue ourselves with a in-house db abstraction layer. The
way the developers handled it and I think the way you guys should as
well is by wrapping find_or_create with a exception handler for
DBD::mysql::st execute failed: Duplicate entry which if triggered makes
it run the find function again. So it would look something like this
assuming you put some flags in to handle that exception at the lower
levels like DBIx::Class::Storage, DBIx::Class::Storage::DBI:: . Of
course its not tested code just an example I don't know how this affects
other storage engines and other implications.

sub find_or_create {
  my $self     = shift;
  my $attrs    = (@_ > 1 && ref $_[$#_] eq 'HASH' ? pop(@_) : {});
  my $hash     = ref $_[0] eq 'HASH' ? shift : {@_};
  my $exists   = $self->find($hash, $attrs);
  if (!$exists)
  {
      # Assuming -1 is returned specifically on Duplicate Key Inserts
      my $id = $self->create($hash);
      if ($id == -1)
      {
	# Reselects and returns ID since it must have been created after
the first find and before the create
	return $self->find($hash, $attrs);
      }
      else
      {
	# Returns the ID since the create worked fine
	return $id;
      }
  }
  else
  {
      return $exists;
  }
}

Let me know if you guys need more clarification on this because I think
it's a bug. By the way DBIx::Class is pretty much everything I wanted in
a DB abstraction layer. I love it!

Thanks,
------------------------------------------
Ali Mesdaq
Security Researcher II
Websense Security Labs
http://www.WebsenseSecurityLabs.com
------------------------------------------