From 0913614d5f0327261500816e816884df46687847 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Thu, 11 Jun 2026 11:05:51 +0200 Subject: [PATCH 1/5] Test pulling pending tenants concurrently --- tests/Etc/pull-worker.php | 34 ++++++++++++++++ tests/PendingTenantsTest.php | 78 ++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 tests/Etc/pull-worker.php diff --git a/tests/Etc/pull-worker.php b/tests/Etc/pull-worker.php new file mode 100644 index 000000000..aed9cb9c9 --- /dev/null +++ b/tests/Etc/pull-worker.php @@ -0,0 +1,34 @@ + ` + * + * Outputs the key of the pulled tenant, or "null" if nothing was pulled. + */ + +require __DIR__ . '/../../vendor/autoload.php'; + +use Stancl\Tenancy\Tests\Etc\Tenant; +use Stancl\Tenancy\Tests\TestCase; + +$startAt = (float) ($argv[1] ?? 0); +$firstOrCreate = ($argv[2] ?? '0') === '1'; + +// createApplication() replays the suite's central-MySQL config without running setUp(), +// so the pending tenants the parent test created survive into this process. +(new class('pull-worker') extends TestCase {})->createApplication(); + +// Wait so that every worker pulls at the same time +if ($startAt > 0.0) { + time_sleep_until($startAt); +} + +$tenant = Tenant::pullPendingFromPool($firstOrCreate); + +fwrite(STDOUT, $tenant?->getKey() ?? 'null'); diff --git a/tests/PendingTenantsTest.php b/tests/PendingTenantsTest.php index b04f8bc47..6892ddb3e 100644 --- a/tests/PendingTenantsTest.php +++ b/tests/PendingTenantsTest.php @@ -5,6 +5,7 @@ use Illuminate\Database\QueryException; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Artisan; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Schema; use Illuminate\Support\Str; @@ -28,6 +29,7 @@ use Stancl\Tenancy\Listeners\BootstrapTenancy; use Stancl\Tenancy\Events\TenancyEnded; use Stancl\Tenancy\Listeners\RevertToCentralContext; +use Symfony\Component\Process\Process; beforeEach($cleanup = function () { Tenant::$extraCustomColumns = []; @@ -126,6 +128,82 @@ expect(Tenant::withPending()->get()->count())->toBe(1); // All tenants }); +/** + * Spawn $count separate PHP processes that all call pullPendingFromPool() at the same + * time and return the keys of pulled tenants to simulate concurrent pulls. + * + * @see tests/Etc/pull-worker.php + */ +function runConcurrentPulls(int $count, bool $firstOrCreate = false): array +{ + $worker = __DIR__ . '/Etc/pull-worker.php'; + + // Shared start instant + $startAt = (string) (microtime(true) + 3.0); + + /** @var Process[] $processes */ + $processes = []; + + for ($i = 0; $i < $count; $i++) { + $process = new Process( + ['php', $worker, $startAt, $firstOrCreate ? '1' : '0'] + ); + $process->start(); + $processes[] = $process; + } + + $pulledTenants = []; + + foreach ($processes as $process) { + $process->wait(); + + expect($process->isSuccessful())->toBeTrue($process->getErrorOutput()); + + $output = trim($process->getOutput()); + + if ($output !== 'null' && $output !== '') { + // If a tenant was pulled, add its key to the results + $pulledTenants[] = $output; + } + } + + return $pulledTenants; +} + +test('concurrent pulls each claim a distinct pending tenant', function (bool $firstOrCreate) { + Tenant::createPending(); + Tenant::createPending(); + Tenant::createPending(); + + expect(Tenant::onlyPending()->count())->toBe(3); + + runConcurrentPulls(8, $firstOrCreate); + + expect(Tenant::onlyPending()->count())->toBe(0); + expect(Tenant::withoutPending()->count())->toBe($firstOrCreate ? 8 : 3); +})->with([ + 'pull pending' => false, + 'pull pending or create' => true, +]); + +test('a failed attribute write rolls back the claim and leaves the tenant pending', function () { + // The claim and the attribute write share one transaction, so if applying the attributes + // fails the claim must roll back and the tenant must stay in the pool. + Schema::table('tenants', function (Blueprint $table) { + $table->string('slug')->nullable()->unique(); + }); + + Tenant::$extraCustomColumns = ['slug']; + + Tenant::create(['slug' => 'taken']); + Tenant::createPending(); + + expect(fn () => Tenant::pullPendingFromPool(false, ['slug' => 'taken'])) + ->toThrow(QueryException::class); + + expect(Tenant::onlyPending()->count())->toBe(1); +}); + test('withoutPending chained with where clauses returns correct results', function () { $tenant = Tenant::create(); $pendingTenant = Tenant::createPending(); From 0995a073c27f8e578f342dc32d5e8d427b05fd06 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Thu, 11 Jun 2026 11:07:18 +0200 Subject: [PATCH 2/5] Handle pulling a single pending tenant concurrently --- src/Database/Concerns/HasPending.php | 53 +++++++++++++++++++--------- 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/src/Database/Concerns/HasPending.php b/src/Database/Concerns/HasPending.php index 04fcccc10..f0d2ea0ee 100644 --- a/src/Database/Concerns/HasPending.php +++ b/src/Database/Concerns/HasPending.php @@ -100,27 +100,46 @@ public static function pullPending(array $attributes = []): Model&Tenant */ public static function pullPendingFromPool(bool $firstOrCreate = false, array $attributes = []): ?Tenant { - $tenant = DB::transaction(function () use ($attributes): ?Tenant { - /** @var (Model&Tenant)|null $tenant */ - $tenant = static::onlyPending()->first(); - - if ($tenant !== null) { - event(new PullingPendingTenant($tenant)); - $tenant->update(array_merge($attributes, [ - 'pending_since' => null, - ])); + // Attempt pulling a pending tenant. + // The loop handles the case where a single tenant is being pulled by multiple processes at the same time. + // If a tenant was pulled by a concurrent process, try pulling the next one in the pool. + while (true) { + /** @var (Model&Tenant)|null $pullCandidate */ + $pullCandidate = static::onlyPending()->first(); + + if ($pullCandidate === null) { + return $firstOrCreate ? static::create($attributes) : null; } - return $tenant; - }); + event(new PullingPendingTenant($pullCandidate)); - if ($tenant === null) { - return $firstOrCreate ? static::create($attributes) : null; - } + $tenant = DB::transaction(function () use ($pullCandidate, $attributes): ?Tenant { + $tenantWasPulled = static::onlyPending() + ->whereKey($pullCandidate->getKey()) + ->update([$pullCandidate->getColumnForQuery('pending_since') => null]) > 0; + + if (! $tenantWasPulled) { + return null; + } + + /** @var Model&Tenant $pulledTenant */ + $pulledTenant = static::findOrFail($pullCandidate->getKey()); + + if (! empty($attributes)) { + $pulledTenant->update($attributes); + } - // Only triggered if a tenant that was pulled from the pool is returned - event(new PendingTenantPulled($tenant)); + return $pulledTenant; + }); - return $tenant; + if ($tenant === null) { + // If another pull claimed this tenant first, try claiming the next one + continue; + } + + event(new PendingTenantPulled($tenant)); + + return $tenant; + } } } From 4b989d87b0946735108922781bb394e918a481d5 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Fri, 12 Jun 2026 10:14:10 +0200 Subject: [PATCH 3/5] Simplify concurrent pull test Remove the worker solution, simulate the concurrent pull by updating pending_since while PullingPendingTenant --- tests/Etc/pull-worker.php | 34 ------------------ tests/PendingTenantsTest.php | 69 ++++++++++-------------------------- 2 files changed, 18 insertions(+), 85 deletions(-) delete mode 100644 tests/Etc/pull-worker.php diff --git a/tests/Etc/pull-worker.php b/tests/Etc/pull-worker.php deleted file mode 100644 index aed9cb9c9..000000000 --- a/tests/Etc/pull-worker.php +++ /dev/null @@ -1,34 +0,0 @@ - ` - * - * Outputs the key of the pulled tenant, or "null" if nothing was pulled. - */ - -require __DIR__ . '/../../vendor/autoload.php'; - -use Stancl\Tenancy\Tests\Etc\Tenant; -use Stancl\Tenancy\Tests\TestCase; - -$startAt = (float) ($argv[1] ?? 0); -$firstOrCreate = ($argv[2] ?? '0') === '1'; - -// createApplication() replays the suite's central-MySQL config without running setUp(), -// so the pending tenants the parent test created survive into this process. -(new class('pull-worker') extends TestCase {})->createApplication(); - -// Wait so that every worker pulls at the same time -if ($startAt > 0.0) { - time_sleep_until($startAt); -} - -$tenant = Tenant::pullPendingFromPool($firstOrCreate); - -fwrite(STDOUT, $tenant?->getKey() ?? 'null'); diff --git a/tests/PendingTenantsTest.php b/tests/PendingTenantsTest.php index 6892ddb3e..40f7e27c1 100644 --- a/tests/PendingTenantsTest.php +++ b/tests/PendingTenantsTest.php @@ -29,7 +29,6 @@ use Stancl\Tenancy\Listeners\BootstrapTenancy; use Stancl\Tenancy\Events\TenancyEnded; use Stancl\Tenancy\Listeners\RevertToCentralContext; -use Symfony\Component\Process\Process; beforeEach($cleanup = function () { Tenant::$extraCustomColumns = []; @@ -128,63 +127,31 @@ expect(Tenant::withPending()->get()->count())->toBe(1); // All tenants }); -/** - * Spawn $count separate PHP processes that all call pullPendingFromPool() at the same - * time and return the keys of pulled tenants to simulate concurrent pulls. - * - * @see tests/Etc/pull-worker.php - */ -function runConcurrentPulls(int $count, bool $firstOrCreate = false): array -{ - $worker = __DIR__ . '/Etc/pull-worker.php'; - - // Shared start instant - $startAt = (string) (microtime(true) + 3.0); - - /** @var Process[] $processes */ - $processes = []; - - for ($i = 0; $i < $count; $i++) { - $process = new Process( - ['php', $worker, $startAt, $firstOrCreate ? '1' : '0'] - ); - $process->start(); - $processes[] = $process; - } - - $pulledTenants = []; - - foreach ($processes as $process) { - $process->wait(); - - expect($process->isSuccessful())->toBeTrue($process->getErrorOutput()); +test('pulling a pending tenant retries when the tenant is claimed concurrently', function () { + Tenant::createPending(); + Tenant::createPending(); - $output = trim($process->getOutput()); + $stolenId = null; - if ($output !== 'null' && $output !== '') { - // If a tenant was pulled, add its key to the results - $pulledTenants[] = $output; + Event::listen(PullingPendingTenant::class, function (PullingPendingTenant $event) use (&$stolenId) { + if ($stolenId !== null) { + return; } - } - return $pulledTenants; -} - -test('concurrent pulls each claim a distinct pending tenant', function (bool $firstOrCreate) { - Tenant::createPending(); - Tenant::createPending(); - Tenant::createPending(); + $stolenId = $event->tenant->id; - expect(Tenant::onlyPending()->count())->toBe(3); + // Steal the tenant like a concurrent process would + Tenant::onlyPending() + ->whereKey($event->tenant->id) + ->update([$event->tenant->getColumnForQuery('pending_since') => null]); + }); - runConcurrentPulls(8, $firstOrCreate); + $pulled = Tenant::pullPendingFromPool(); - expect(Tenant::onlyPending()->count())->toBe(0); - expect(Tenant::withoutPending()->count())->toBe($firstOrCreate ? 8 : 3); -})->with([ - 'pull pending' => false, - 'pull pending or create' => true, -]); + expect($pulled)->not()->toBeNull(); + expect($pulled->id)->not()->toBe($stolenId); // Stolen tenant was skipped, the next one was claimed by the pull + expect(Tenant::onlyPending()->count())->toBe(0); // Both tenants claimed +}); test('a failed attribute write rolls back the claim and leaves the tenant pending', function () { // The claim and the attribute write share one transaction, so if applying the attributes From 698589edbdc6f44d142ed35d8044045775d3d430 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Fri, 12 Jun 2026 11:47:39 +0200 Subject: [PATCH 4/5] Add comments for clarity (re-fetching and event firing) --- src/Database/Concerns/HasPending.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Database/Concerns/HasPending.php b/src/Database/Concerns/HasPending.php index f0d2ea0ee..922c6bdf5 100644 --- a/src/Database/Concerns/HasPending.php +++ b/src/Database/Concerns/HasPending.php @@ -111,6 +111,9 @@ public static function pullPendingFromPool(bool $firstOrCreate = false, array $a return $firstOrCreate ? static::create($attributes) : null; } + // Fired before the claim, so it can fire once per attempt, including for a candidate + // that ends up being claimed by a concurrent process (in which case the loop retries). + // PendingTenantPulled (below) fires exactly once, for the pulled tenant. event(new PullingPendingTenant($pullCandidate)); $tenant = DB::transaction(function () use ($pullCandidate, $attributes): ?Tenant { @@ -122,6 +125,8 @@ public static function pullPendingFromPool(bool $firstOrCreate = false, array $a return null; } + // The tenant's pending_since was just cleared, and e.g. a PullingPendingTenant listener + // may have made changes to the tenant, so re-fetch it to get it in the correct state. /** @var Model&Tenant $pulledTenant */ $pulledTenant = static::findOrFail($pullCandidate->getKey()); From e5234daed4186d7e047d73d293e653a5a5278693 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Fri, 12 Jun 2026 12:39:35 +0200 Subject: [PATCH 5/5] Improve wording, add comments in the pull rollback test --- tests/PendingTenantsTest.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/PendingTenantsTest.php b/tests/PendingTenantsTest.php index 40f7e27c1..c99607281 100644 --- a/tests/PendingTenantsTest.php +++ b/tests/PendingTenantsTest.php @@ -153,9 +153,9 @@ expect(Tenant::onlyPending()->count())->toBe(0); // Both tenants claimed }); -test('a failed attribute write rolls back the claim and leaves the tenant pending', function () { - // The claim and the attribute write share one transaction, so if applying the attributes - // fails the claim must roll back and the tenant must stay in the pool. +test('the pull is rolled back and the tenant stays in the pool if setting attributes fails', function () { + // Pulling a tenant and setting its attributes happen in one transaction, + // so if setting the attributes fails, the whole pull rolls back and the tenant stays in the pool. Schema::table('tenants', function (Blueprint $table) { $table->string('slug')->nullable()->unique(); }); @@ -165,9 +165,11 @@ Tenant::create(['slug' => 'taken']); Tenant::createPending(); + // During the pull, set slug to 'taken', which is already used by another tenant to make the attribute update throw expect(fn () => Tenant::pullPendingFromPool(false, ['slug' => 'taken'])) ->toThrow(QueryException::class); + // The pull rolled back, so the tenant is still pending expect(Tenant::onlyPending()->count())->toBe(1); });