Skip to content

Commit

Permalink
[1.x] Improve unique constraint handling for high traffic applications (
Browse files Browse the repository at this point in the history 
#104)

* Ensure `set` can handle conflicts

* Run update in single query

* Update tests

* Improve unique constraint handling

* Formatting

* Update attempts
  • Loading branch information
@timacdonald
timacdonald authored May 21, 2024
1 parent 83178d7 commit 2226b13
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 35 deletions.
70 changes: 48 additions & 22 deletions src/Drivers/DatabaseDriver.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@
use Illuminate\Contracts\Events\Dispatcher;
use Illuminate\Database\Connection;
use Illuminate\Database\DatabaseManager;
use Illuminate\Database\UniqueConstraintViolationException;
use Illuminate\Support\Carbon;
use Illuminate\Support\Collection;
use Laravel\Pennant\Contracts\CanListStoredFeatures;
use Laravel\Pennant\Contracts\Driver;
use Laravel\Pennant\Events\UnknownFeatureResolved;
use Laravel\Pennant\Feature;
use RuntimeException;
use stdClass;

class DatabaseDriver implements CanListStoredFeatures, Driver
Expand Down Expand Up @@ -58,6 +60,13 @@ class DatabaseDriver implements CanListStoredFeatures, Driver
*/
protected $unknownFeatureValue;

/**
* The current retry depth for retrieving values from the database.
*
* @var int
*/
protected $retryDepth = 0;

/**
* The name of the "created at" column.
*
Expand Down Expand Up @@ -170,11 +179,23 @@ public function getAll($features): array
if ($inserts->isNotEmpty()) {
$now = Carbon::now();

$this->newQuery()->insert($inserts->map(fn ($insert) => [
...$insert,
static::CREATED_AT => $now,
static::UPDATED_AT => $now,
])->all());
try {
$this->newQuery()->insert($inserts->map(fn ($insert) => [
...$insert,
static::CREATED_AT => $now,
static::UPDATED_AT => $now,
])->all());
} catch (UniqueConstraintViolationException $e) {
if ($this->retryDepth === 2) {
throw new RuntimeException('Unable to insert feature values into the database.', previous: $e);
}

$this->retryDepth++;

return $this->getAll($features);
} finally {
$this->retryDepth = 0;
}
}

return $results;
Expand All @@ -197,7 +218,19 @@ public function get($feature, $scope): mixed
return false;
}

$this->insert($feature, $scope, $value);
try {
$this->insert($feature, $scope, $value);
} catch (UniqueConstraintViolationException $e) {
if ($this->retryDepth === 1) {
throw new RuntimeException('Unable to insert feature value from the database.', previous: $e);
}

$this->retryDepth++;

return $this->get($feature, $scope);
} finally {
$this->retryDepth = 0;
}

return $value;
});
Expand Down Expand Up @@ -245,9 +278,13 @@ protected function resolveValue($feature, $scope)
*/
public function set($feature, $scope, $value): void
{
if (! $this->update($feature, $scope, $value)) {
$this->insert($feature, $scope, $value);
}
$this->newQuery()->upsert([
'name' => $feature,
'scope' => Feature::serializeScope($scope),
'value' => json_encode($value, flags: JSON_THROW_ON_ERROR),
static::CREATED_AT => $now = Carbon::now(),
static::UPDATED_AT => $now,
], uniqueBy: ['name', 'scope'], update: ['value', 'updated_at']);
}

/**
Expand Down Expand Up @@ -276,24 +313,13 @@ public function setForAllScopes($feature, $value): void
*/
protected function update($feature, $scope, $value)
{
$exists = $this->newQuery()
->where('name', $feature)
->where('scope', $serialized = Feature::serializeScope($scope))
->exists();

if (! $exists) {
return false;
}

$this->newQuery()
return (bool) $this->newQuery()
->where('name', $feature)
->where('scope', $serialized)
->where('scope', Feature::serializeScope($scope))
->update([
'value' => json_encode($value, flags: JSON_THROW_ON_ERROR),
static::UPDATED_AT => Carbon::now(),
]);

return true;
}

/**
Expand Down
26 changes: 13 additions & 13 deletions tests/Feature/DatabaseDriverTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public function test_it_can_register_complex_values()
$this->assertTrue($active);
$this->assertSame('foo', $value);

$this->assertCount(4, DB::getQueryLog());
$this->assertCount(3, DB::getQueryLog());
}

public function test_it_caches_state_after_resolving()
Expand Down Expand Up @@ -162,7 +162,7 @@ public function test_it_can_programatically_activate_and_deativate_features()
Feature::activate('foo');
$this->assertTrue(Feature::active('foo'));

$this->assertCount(6, DB::getQueryLog());
$this->assertCount(3, DB::getQueryLog());
}

public function test_it_dispatches_events_when_checking_known_features()
Expand Down Expand Up @@ -201,7 +201,7 @@ public function test_it_can_activate_and_deactivate_several_features_at_once()
$this->assertTrue(Feature::active('bar'));
$this->assertTrue(Feature::active('bar'));

$this->assertCount(13, DB::getQueryLog());
$this->assertCount(7, DB::getQueryLog());
}

public function test_it_can_check_if_multiple_features_are_active_at_once()
Expand All @@ -218,7 +218,7 @@ public function test_it_can_check_if_multiple_features_are_active_at_once()
$this->assertTrue(Feature::allAreActive(['foo', 'bar']));
$this->assertFalse(Feature::allAreActive(['foo', 'bar', 'baz']));

$this->assertCount(7, DB::getQueryLog());
$this->assertCount(4, DB::getQueryLog());
}

public function test_it_can_scope_features()
Expand Down Expand Up @@ -252,7 +252,7 @@ public function test_it_can_activate_and_deactivate_features_with_scope()
$this->assertTrue(Feature::for($first)->active('foo'));
$this->assertFalse(Feature::for($second)->active('foo'));

$this->assertCount(4, DB::getQueryLog());
$this->assertCount(3, DB::getQueryLog());
}

public function test_it_can_activate_and_deactivate_features_for_multiple_scope_at_once()
Expand All @@ -268,7 +268,7 @@ public function test_it_can_activate_and_deactivate_features_for_multiple_scope_
$this->assertTrue(Feature::for($second)->active('foo'));
$this->assertFalse(Feature::for($third)->active('foo'));

$this->assertCount(6, DB::getQueryLog());
$this->assertCount(4, DB::getQueryLog());
}

public function test_it_can_activate_and_deactivate_multiple_features_for_multiple_scope_at_once()
Expand All @@ -289,7 +289,7 @@ public function test_it_can_activate_and_deactivate_multiple_features_for_multip
$this->assertTrue(Feature::for($second)->active('bar'));
$this->assertFalse(Feature::for($third)->active('bar'));

$this->assertCount(12, DB::getQueryLog());
$this->assertCount(8, DB::getQueryLog());
}

public function test_it_can_check_multiple_features_for_multiple_scope_at_once()
Expand All @@ -309,7 +309,7 @@ public function test_it_can_check_multiple_features_for_multiple_scope_at_once()
$this->assertFalse(Feature::for([$second, $third])->allAreActive(['foo', 'bar']));
$this->assertFalse(Feature::for([$first, $second, $third])->allAreActive(['foo', 'bar']));

$this->assertCount(10, DB::getQueryLog());
$this->assertCount(6, DB::getQueryLog());
}

public function test_null_is_same_as_global()
Expand All @@ -318,7 +318,7 @@ public function test_null_is_same_as_global()

$this->assertTrue(Feature::for(null)->active('foo'));

$this->assertCount(2, DB::getQueryLog());
$this->assertCount(1, DB::getQueryLog());
}

public function test_it_sees_null_and_empty_string_as_different_things()
Expand All @@ -335,7 +335,7 @@ public function test_it_sees_null_and_empty_string_as_different_things()
$this->assertFalse(Feature::for(null)->active('bar'));
$this->assertFalse(Feature::active('bar'));

$this->assertCount(6, DB::getQueryLog());
$this->assertCount(4, DB::getQueryLog());
}

public function test_scope_can_be_strings_like_email_addresses()
Expand All @@ -345,7 +345,7 @@ public function test_scope_can_be_strings_like_email_addresses()
$this->assertFalse(Feature::for('[email protected]')->active('foo'));
$this->assertTrue(Feature::for('[email protected]')->active('foo'));

$this->assertCount(3, DB::getQueryLog());
$this->assertCount(2, DB::getQueryLog());
}

public function test_it_can_handle_feature_scopeable_objects()
Expand All @@ -364,7 +364,7 @@ public function toFeatureIdentifier($driver): mixed
$this->assertTrue(Feature::for('[email protected]')->active('foo'));
$this->assertTrue(Feature::for($scopeable())->active('foo'));

$this->assertCount(3, DB::getQueryLog());
$this->assertCount(2, DB::getQueryLog());
}

public function test_it_serializes_eloquent_models()
Expand Down Expand Up @@ -617,7 +617,7 @@ public function test_missing_results_are_inserted_on_load()
Feature::for('[email protected]')->activate('foo', 99);
Feature::for(['[email protected]', '[email protected]', '[email protected]'])->load(['foo', 'bar']);

$this->assertCount(4, DB::getQueryLog());
$this->assertCount(3, DB::getQueryLog());
$this->assertDatabaseHas('features', [
'name' => 'foo',
'scope' => '[email protected]',
Expand Down

0 comments on commit 2226b13

Please sign in to comment.