From 86845b7158f21733f60f51d2b47e542f3b2be54e Mon Sep 17 00:00:00 2001 From: romanetar Date: Wed, 21 Oct 2026 18:46:01 +0200 Subject: [PATCH 1/3] fix(attendees): clear the EntityManager between pages and bound the timeout of SynchAllAttendeesStatus The attendee status resynch kept every processed attendee in the Doctrine identity map for the whole run and relied on the worker default timeout. Clear the manager (fetched from the registry on each page, since a failed transaction replaces it), log memory usage per page, set an explicit job timeout and acquire the per-summit lock atomically with a TTL so a killed run does not block later ones. --- app/Jobs/SynchAllAttendeesStatus.php | 3 +++ app/Services/Model/AttendeeService.php | 28 ++++++++++++++++++++++++-- tests/AttendeeServiceTest.php | 24 ++++++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/app/Jobs/SynchAllAttendeesStatus.php b/app/Jobs/SynchAllAttendeesStatus.php index d5a4792d9a..b0fa8c9c35 100644 --- a/app/Jobs/SynchAllAttendeesStatus.php +++ b/app/Jobs/SynchAllAttendeesStatus.php @@ -31,6 +31,9 @@ final class SynchAllAttendeesStatus implements ShouldQueue { public $tries = 1; + // 2 hours, bounded explicitly instead of relying on the worker default + public $timeout = 7200; + use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; /** diff --git a/app/Services/Model/AttendeeService.php b/app/Services/Model/AttendeeService.php index b0dee3d57f..451676b413 100644 --- a/app/Services/Model/AttendeeService.php +++ b/app/Services/Model/AttendeeService.php @@ -24,6 +24,8 @@ use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\Log; +use LaravelDoctrine\ORM\Facades\Registry; +use models\utils\SilverstripeBaseModel; use libs\utils\ITransactionService; use models\exceptions\EntityNotFoundException; use models\exceptions\ValidationException; @@ -57,6 +59,11 @@ */ final class AttendeeService extends AbstractService implements IAttendeeService { + /** + * seconds, same bound as SynchAllAttendeesStatus::$timeout + */ + const ResynchAttendeesStatusLockTTL = 7200; + /** * @var ISummitAttendeeRepository */ @@ -1129,13 +1136,13 @@ public function resynchAttendeesStatusBySummit(int $summit_id): void Log::debug(sprintf("AttendeeService::resynchAttendeesStatusBySummit summit id %s", $summit_id)); $key = sprintf("AttendeeService::resynchAttendeesStatusBySummit::%s", $summit_id); - if (Cache::has($key)) { + // atomic acquire; the TTL lets a killed run (no finally) release the lock by itself + if (!Cache::add($key, $key, self::ResynchAttendeesStatusLockTTL)) { Log::debug(sprintf("AttendeeService::resynchAttendeesStatusBySummit summit id %s already running", $summit_id)); return; } try { - Cache::add($key, $key); $summit = $this->summit_repository->getById($summit_id); if (!$summit instanceof Summit) throw new EntityNotFoundException(sprintf("Summit %s not found", $summit_id)); @@ -1179,6 +1186,23 @@ public function resynchAttendeesStatusBySummit(int $summit_id): void } } ++$current_page; + + // release the identity map between pages; the manager is fetched from the registry on + // every page because DoctrineTransactionService replaces it after a failed transaction + $em = Registry::getManager(SilverstripeBaseModel::EntityManager); + if ($em->isOpen()) + $em->clear(); + + Log::debug + ( + sprintf + ( + "AttendeeService::resynchAttendeesStatusBySummit summit id %s processed %s attendees so far, memory usage %s bytes", + $summit_id, + $count, + memory_get_usage(true) + ) + ); } while (true); } finally { Cache::forget($key); diff --git a/tests/AttendeeServiceTest.php b/tests/AttendeeServiceTest.php index 7ff1a67cf8..a66b5c239b 100644 --- a/tests/AttendeeServiceTest.php +++ b/tests/AttendeeServiceTest.php @@ -17,6 +17,7 @@ use App\Models\Foundation\Main\IGroup; use App\Services\Model\IAttendeeService; use Illuminate\Support\Facades\App; +use Illuminate\Support\Facades\Cache; use LaravelDoctrine\ORM\Facades\EntityManager; use models\summit\Summit; use models\summit\SummitAttendeeBadge; @@ -54,6 +55,29 @@ public function testRedeemPromoCodes(){ $service->updateRedeemedPromoCodes($summit); } + public function testResynchAttendeesStatusBySummitReleasesLock() { + $service = App::make(IAttendeeService::class); + $key = sprintf("AttendeeService::resynchAttendeesStatusBySummit::%s", self::$summit->getId()); + + $service->resynchAttendeesStatusBySummit(self::$summit->getId()); + + $this->assertFalse(Cache::has($key)); + } + + public function testResynchAttendeesStatusBySummitSkipsWhenLocked() { + $service = App::make(IAttendeeService::class); + $key = sprintf("AttendeeService::resynchAttendeesStatusBySummit::%s", self::$summit->getId()); + Cache::add($key, $key); + + try { + $service->resynchAttendeesStatusBySummit(self::$summit->getId()); + // the run that owns the lock must not be released by a skipped one + $this->assertTrue(Cache::has($key)); + } finally { + Cache::forget($key); + } + } + public function testSendAllAttendeeTickets() { $service = App::make(IAttendeeService::class); From 0b736d41600f6871051f0d1e64ca77b15c939cb9 Mon Sep 17 00:00:00 2001 From: romanetar Date: Wed, 21 Oct 2026 19:21:00 +0200 Subject: [PATCH 2/3] fix(attendees): use an owner-aware lock for the attendee status resynch Cache::forget in the finally could remove the lock of a newer run when a run outlived the TTL (e.g. RecalculateAttendeesStatusCommand has no timeout). Use Cache::lock and release the handle held by this run instead. --- app/Services/Model/AttendeeService.php | 8 +++++--- tests/AttendeeServiceTest.php | 28 ++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/app/Services/Model/AttendeeService.php b/app/Services/Model/AttendeeService.php index 451676b413..bac370f567 100644 --- a/app/Services/Model/AttendeeService.php +++ b/app/Services/Model/AttendeeService.php @@ -1136,8 +1136,10 @@ public function resynchAttendeesStatusBySummit(int $summit_id): void Log::debug(sprintf("AttendeeService::resynchAttendeesStatusBySummit summit id %s", $summit_id)); $key = sprintf("AttendeeService::resynchAttendeesStatusBySummit::%s", $summit_id); - // atomic acquire; the TTL lets a killed run (no finally) release the lock by itself - if (!Cache::add($key, $key, self::ResynchAttendeesStatusLockTTL)) { + // owner-aware lock: the TTL lets a killed run (no finally) release it by itself, and release() + // in the finally only frees it if this run still owns it (a longer run can outlive the TTL) + $lock = Cache::lock($key, self::ResynchAttendeesStatusLockTTL); + if (!$lock->get()) { Log::debug(sprintf("AttendeeService::resynchAttendeesStatusBySummit summit id %s already running", $summit_id)); return; } @@ -1205,7 +1207,7 @@ public function resynchAttendeesStatusBySummit(int $summit_id): void ); } while (true); } finally { - Cache::forget($key); + $lock->release(); } Log::debug(sprintf("AttendeeService::resynchAttendeesStatusBySummit summit id %s processed %s attendees", $summit_id, $count)); diff --git a/tests/AttendeeServiceTest.php b/tests/AttendeeServiceTest.php index a66b5c239b..f1bc128ea7 100644 --- a/tests/AttendeeServiceTest.php +++ b/tests/AttendeeServiceTest.php @@ -61,20 +61,40 @@ public function testResynchAttendeesStatusBySummitReleasesLock() { $service->resynchAttendeesStatusBySummit(self::$summit->getId()); - $this->assertFalse(Cache::has($key)); + // the lock is free again: a new run can acquire it + $lock = Cache::lock($key, 60); + $this->assertTrue($lock->get()); + $lock->release(); + } + + public function testResynchAttendeesStatusBySummitDoesNotReleaseAnotherRunsLock() { + $service = App::make(IAttendeeService::class); + $key = sprintf("AttendeeService::resynchAttendeesStatusBySummit::%s", self::$summit->getId()); + + // a lock held by a different owner (a newer run, after this one's TTL expired) must survive + $other = Cache::lock($key, 60, 'other-run'); + $this->assertTrue($other->get()); + + try { + $service->resynchAttendeesStatusBySummit(self::$summit->getId()); + $this->assertTrue($other->isOwnedByCurrentProcess()); + } finally { + $other->release(); + } } public function testResynchAttendeesStatusBySummitSkipsWhenLocked() { $service = App::make(IAttendeeService::class); $key = sprintf("AttendeeService::resynchAttendeesStatusBySummit::%s", self::$summit->getId()); - Cache::add($key, $key); + $lock = Cache::lock($key, 60); + $this->assertTrue($lock->get()); try { $service->resynchAttendeesStatusBySummit(self::$summit->getId()); // the run that owns the lock must not be released by a skipped one - $this->assertTrue(Cache::has($key)); + $this->assertTrue($lock->isOwnedByCurrentProcess()); } finally { - Cache::forget($key); + $lock->release(); } } From 5452f6d8d04741ac0716c2f7645a7649dca20f9d Mon Sep 17 00:00:00 2001 From: romanetar Date: Wed, 21 Oct 2026 19:45:15 +0200 Subject: [PATCH 3/3] test(attendees): exercise the lock handover in the resynch owner-aware lock test The previous test acquired the other owner's lock before calling the service, so the service returned early and never reached the release in its finally. Take the lock over mid-run instead, through a wrapping transaction service, so the test fails if the run frees a lock it no longer owns. --- tests/AttendeeServiceTest.php | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/tests/AttendeeServiceTest.php b/tests/AttendeeServiceTest.php index f1bc128ea7..c2ece3289e 100644 --- a/tests/AttendeeServiceTest.php +++ b/tests/AttendeeServiceTest.php @@ -15,9 +15,13 @@ use App\Jobs\Emails\SummitAttendeeAllTicketsEditionEmail; use App\Jobs\Emails\SummitAttendeeRegistrationIncompleteReminderEmail; use App\Models\Foundation\Main\IGroup; +use App\Services\Model\AttendeeService; use App\Services\Model\IAttendeeService; +use Closure; +use Illuminate\Contracts\Cache\Lock; use Illuminate\Support\Facades\App; use Illuminate\Support\Facades\Cache; +use libs\utils\ITransactionService; use LaravelDoctrine\ORM\Facades\EntityManager; use models\summit\Summit; use models\summit\SummitAttendeeBadge; @@ -68,15 +72,31 @@ public function testResynchAttendeesStatusBySummitReleasesLock() { } public function testResynchAttendeesStatusBySummitDoesNotReleaseAnotherRunsLock() { - $service = App::make(IAttendeeService::class); $key = sprintf("AttendeeService::resynchAttendeesStatusBySummit::%s", self::$summit->getId()); - // a lock held by a different owner (a newer run, after this one's TTL expired) must survive + // while the run is in progress (first transaction, lock already acquired), simulate its TTL + // expiring and a newer run taking the lock; the release() in the finally of the first run + // must leave the newer run's lock in place $other = Cache::lock($key, 60, 'other-run'); - $this->assertTrue($other->get()); + $tx_service = new class(App::make(ITransactionService::class), $key, $other) implements ITransactionService { + public bool $taken = false; + + public function __construct(private ITransactionService $inner, private string $key, private Lock $other) {} + + public function transaction(Closure $callback, int $isolationLevel = 2) + { + if (!$this->taken) { + Cache::lock($this->key)->forceRelease(); + $this->taken = $this->other->get(); + } + return $this->inner->transaction($callback, $isolationLevel); + } + }; + $service = App::make(AttendeeService::class, ['tx_service' => $tx_service]); try { $service->resynchAttendeesStatusBySummit(self::$summit->getId()); + $this->assertTrue($tx_service->taken); $this->assertTrue($other->isOwnedByCurrentProcess()); } finally { $other->release();