pennant icon indicating copy to clipboard operation
pennant copied to clipboard

[1.x] Improve unique constraint handling for high traffic applications

Open timacdonald opened this issue 9 months ago • 0 comments

fixes #101

Problem

Laravel Pennant implements a unique constraint that can cause race conditions:

https://github.com/laravel/pennant/blob/83178d76f41d45276da9ce37cf7d76c9f5b28945/database/migrations/2022_11_01_000001_create_features_table.php#L23

The race condition can be illustrated with the following application setup:

echo "Create project:"
composer create-project laravel/laravel pennant-race-condition
cd pennant-race-condition
composer require laravel/pennant
php artisan vendor:publish --tag=pennant-migrations
sed -i "s/DB_CONNECTION=sqlite/DB_CONNECTION=mysql/" .env
sed -i "s/# DB_DATABASE=laravel/DB_DATABASE=pennant_race_condition/" .env
php artisan migrate --force

echo "Create command to invoke:"
echo "<?php

use Illuminate\Foundation\Inspiring;
use Illuminate\Support\Facades\Artisan;
use Laravel\Pennant\Feature;

Artisan::command('dev', function () {
    \$name = 'foo-'.time();

    Feature::define(\$name, fn () => 'okay');

    echo Feature::value('foo-'.time()).PHP_EOL;
});" > routes/console.php

echo "Create script that will invoke the command simulating high traffic:"
echo "php artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nwait\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nwait\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nwait\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nwait\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nwait\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nwait\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nwait\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nwait\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nwait\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nphp artisan dev &\nwait" > script.sh

To simulate the race condition, you may invoke the script:

bash script.sh

Solution

We now retry when hitting a UniqueConstraintViolationException. For a single feature , we only retry once, i.e., two total attempts. For several individual features, via getAll, we retry twice, i.e., 3 total attempts.

This is similar to how the framework handles these internally with createOrFirst.

Why not a cache lock

For indivdual features, we could use a cache lock.

When retrieving several features at a time, like you might do with Feature::loadMissing([/* ... */]); to avoid n+1 queries, a cache lock becomes complicated.

We would need an indivudal cache lock for each feature you are loading. We would need to aquire the cache lock before retrieving, which means every single feature retrievial now requires a cache lock, even if it will never result in a race condition.

If you are using the database driver, now you have increased the queries required to retrieve a feature.

I feel that simply retrying for those rare cases where a race condition will occur is the best way forward. It means 99% of retrievals happen with not speed impact and there is a small cost when a race condition happens to occur.

timacdonald avatar May 21 '24 00:05 timacdonald