StSarc
commented
5 months ago @timacdonald This might be the wrong place to ask, but since it's related, asking here (please let me know if you prefer me creating a issue instead, I'll do that right away)
Can we have insertAll function similar to the insert function present in the databaseDriver already instead of having the query explicit in the getAll function for inserting all?
If it makes sense, I can raise a PR for the same.
This will primarily help with overriding functions in DatabaseDriver. Currently, I just need to override the "writes" to the DB. But, since we have a write inside the getAll function, I have to override the getAll function too unnecessarily.
Related issue by someone else: https://github.com/laravel/pennant/issues/100
timacdonald
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:
To simulate the race condition, you may invoke the script:
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, viagetAll, 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.