Atomic Updates with DBIx::Class
As we're building Tau Station, a narrative sci-fi MMORPG written in Perl, we've encountered any of a number of challenges, not the least of which is that this is a very "write-heavy" application. Further, because it's a universe where people interact, there's often no clear isolation between what you can change and what I can change. Thus, there's a lot of writing to the database and often to the same data!
By now you've probably heard of the Object-Relational Impedance Mismatch, which is just a fancy way of saying "collections of objects and databases aren't the same thing."
One problem which is particularly difficult is handling "syndicate credits". Syndicates ("guilds" in other MMORPGs) can tax their members and every time a member gets credits deposited in their bank account, a certain percentage goes to the syndicate. So let's say two syndicate members each earn 100 credits at the same time, paying 10% tax to their syndicates. It can look like this:
- Process #1: Read how many credits a given syndicate has.
- Process #2: Read how many credits a syndicate has.
- Process #1: Set syndicate credits = credits + 10
- Process #2: Set syndicate credits = credits + 10
Logically, the syndicate should end up with an extra 20 credits. But if you do a naïve:
$syndicate->update({ credits => $credits + $tax });
... you can easily wind up with 10 credits because the "Process #2" read the total credits before "Process #1" wrote them out. Fortunately, there's an easy fix to this.
We built a system that allows us to write declarative Perl to describe these behaviors and this system does a "select for update" on affected rows when it starts. Thus, if I perform an action that impacts my syndicate, the syndicate record is locked until I'm done.
That turned out to be problematic because with many syndicate members, many people were trying to get a lock on the syndicate record, even if they weren't using it! We now have a simpler solution that we've dropped in our dbic base class:
=head2 C<atomic_increment>
$syndicate->atomic_increment(
{
experience => $added_xp,
credits => $added_credits,
}
);
This method takes a hashref and will increase the value of each field (key) by
its value. For the above, it is equivalent to:
UPDATE syndicate
SET experience = experience + $added_xp
credits = credits + $added_credits
WHERE syndicate_id = ?
This ensures that our update will be against the current value of the field
and not just against whatever value our object has when it was created (in
other words, if something else changes those values, this method is safer).
Note that this will generate both an C<UPDATE> and a C<SELECT>.
=cut
sub atomic_increment ( $self, $fields ) {
my %fields = %$fields; # shallow copy
my $dbh = $self->result_source->schema->storage->dbh;
foreach my $field ( keys %fields ) {
my $increment = $fields{$field};
# paranoia. This should never come from untrusted data, but just in
# case ... (thanks Mark!)
my $quoted_field = $dbh->quote_identifier($field);
# if you want subtraction, pass in a negative number
$fields{$field} = \[ "$quoted_field + ?", $increment ];
}
$self->update( \%fields );
# if we don't do this, the values of each field will be the numerical
# value of the reference
$self->discard_changes;
}
Note that because the value we're using is a reference and not a string, DBIC knows that we want to use that array reference to generate literal SQL.
Yes, that's two extra database hits we didn't have before (but we have a potential solution to that, too), but it allows us to forego most locks. And because the SQL update is credits = credits + $added_credits
instead of credits = $new_value
, we don't have to worry that our dbic object grabbed stale data.
If you want this quality for your code, don't forget to drop me a line. We do awesome things with DBIx::Class
, Catalyst
, and other technologies. We'll fix your database while we're at it.
Shouldn't you be using
quote_identifier
or somesuch on that field name?Thanks, Mark. I've updated the code to reflect that!
Having no untrusted data going to this method isn't a good reason to skip that basic security check.
(For those following along at home and don't know what this is about, it's to stop a class of SQL injection attacks against column names. Those can't use placeholders, so quoting is required).
Seems to me you have not "resolved" the lock problem, you have just postponed it to the database itself. If you are issuing two concurrent updates on the same row, the database will use a lock (or MVCC or alike) to solve the conflicts, so depending on your transaction isolation level and the way your database manages such situations, you could end up with locked process too.
This would make a nice addition to DBIx::Class::Helpers!