diff --git a/app/Http/Controllers/Api/ApiV1Controller.php b/app/Http/Controllers/Api/ApiV1Controller.php index 2fe76ad64..ce745d244 100644 --- a/app/Http/Controllers/Api/ApiV1Controller.php +++ b/app/Http/Controllers/Api/ApiV1Controller.php @@ -3252,6 +3252,11 @@ class ApiV1Controller extends Controller return in_array('public', $filter->context); }); + + // Null when no warm was attempted, true when this request ran one, + // false when it lost the lock to a request running one right now. + $warmRan = null; + if ($local && $remote) { $feed = Status::select( 'id', @@ -3282,11 +3287,15 @@ class ApiV1Controller extends Controller ->values() ->toArray(); } elseif ($remote && ! $local) { - if (NetworkTimelineService::count() === 0) { - Cache::lock('api:v1:timelines:network:cache_check:lock', 10)->get(function () { - if (NetworkTimelineService::count() === 0) { + if (NetworkTimelineService::count() === 0 && NetworkTimelineService::shouldWarmEmpty()) { + $warmRan = Cache::lock('api:v1:timelines:network:cache_check:lock', 10)->get(function () { + if (NetworkTimelineService::count() === 0 && NetworkTimelineService::shouldWarmEmpty()) { NetworkTimelineService::warmCache(true); } + + // Returning a value is what makes a lost lock (false) + // distinguishable from a warm that ran. + return true; }); } @@ -3298,11 +3307,13 @@ class ApiV1Controller extends Controller $feed = NetworkTimelineService::get(0, $limit + 5); } } else { - if (PublicTimelineService::count() === 0) { - Cache::lock('api:v1:timelines:public:cache_check:lock', 10)->get(function () { - if (PublicTimelineService::count() === 0) { + if (PublicTimelineService::count() === 0 && PublicTimelineService::shouldWarmEmpty()) { + $warmRan = Cache::lock('api:v1:timelines:public:cache_check:lock', 10)->get(function () { + if (PublicTimelineService::count() === 0 && PublicTimelineService::shouldWarmEmpty()) { PublicTimelineService::warmCache(true, 400); } + + return true; }); } diff --git a/app/Http/Controllers/PublicApiController.php b/app/Http/Controllers/PublicApiController.php index 5fa15b740..4a14c6c44 100644 --- a/app/Http/Controllers/PublicApiController.php +++ b/app/Http/Controllers/PublicApiController.php @@ -260,11 +260,13 @@ class PublicApiController extends Controller $user = $request->user(); $filtered = $user ? UserFilterService::filters($user->profile_id) : []; - if (PublicTimelineService::count() === 0) { + if (PublicTimelineService::count() === 0 && PublicTimelineService::shouldWarmEmpty()) { Cache::lock('api:v1:timelines:public:cache_check:lock', 10)->get(function () { - if (PublicTimelineService::count() === 0) { + if (PublicTimelineService::count() === 0 && PublicTimelineService::shouldWarmEmpty()) { PublicTimelineService::warmCache(true, 400); } + + return true; }); } @@ -465,11 +467,13 @@ class PublicApiController extends Controller $filtered = $user ? UserFilterService::filters($user->profile_id) : []; - if (NetworkTimelineService::count() === 0) { + if (NetworkTimelineService::count() === 0 && NetworkTimelineService::shouldWarmEmpty()) { Cache::lock('api:v1:timelines:network:cache_check:lock', 10)->get(function () { - if (NetworkTimelineService::count() === 0) { + if (NetworkTimelineService::count() === 0 && NetworkTimelineService::shouldWarmEmpty()) { NetworkTimelineService::warmCache(true); } + + return true; }); } diff --git a/app/Services/NetworkTimelineService.php b/app/Services/NetworkTimelineService.php index 457f4606a..2aff14c12 100644 --- a/app/Services/NetworkTimelineService.php +++ b/app/Services/NetworkTimelineService.php @@ -24,6 +24,11 @@ class NetworkTimelineService return self::setShouldWarmEmpty(self::CACHE_KEY); } + public static function markEmptyWarmed(): void + { + self::setMarkEmptyWarmed(self::CACHE_KEY, self::EMPTY_WARM_INTERVAL); + } + public static function clearEmptyWarmed(): void { self::setClearEmptyWarmed(self::CACHE_KEY); @@ -126,6 +131,10 @@ class NetworkTimelineService self::add($id); } + if ($ids->count() === 0) { + self::markEmptyWarmed(); + } + return 1; } diff --git a/app/Services/PublicTimelineService.php b/app/Services/PublicTimelineService.php index 5a87ec9f5..9db5a5948 100644 --- a/app/Services/PublicTimelineService.php +++ b/app/Services/PublicTimelineService.php @@ -27,6 +27,11 @@ class PublicTimelineService return self::setShouldWarmEmpty(self::CACHE_KEY); } + public static function markEmptyWarmed(): void + { + self::setMarkEmptyWarmed(self::CACHE_KEY, self::EMPTY_WARM_INTERVAL); + } + public static function clearEmptyWarmed(): void { self::setClearEmptyWarmed(self::CACHE_KEY); @@ -105,6 +110,8 @@ class PublicTimelineService if ($added === 0) { Redis::del(self::CACHE_KEY); + Redis::del($tmpKey); + self::markEmptyWarmed(); return 1; } @@ -115,6 +122,10 @@ class PublicTimelineService Redis::rename($tmpKey, self::CACHE_KEY); + // The publish is a rename rather than a write through add(), so + // the automatic eviction never fires for a rebuild. + self::clearEmptyWarmed(); + return 1; } diff --git a/tests/Feature/Timeline/PublicNetworkEmptyWarmTest.php b/tests/Feature/Timeline/PublicNetworkEmptyWarmTest.php new file mode 100644 index 000000000..8c8e3751c --- /dev/null +++ b/tests/Feature/Timeline/PublicNetworkEmptyWarmTest.php @@ -0,0 +1,131 @@ + false]); + Redis::del(PublicTimelineService::CACHE_KEY); + Redis::del(NetworkTimelineService::CACHE_KEY); +}); + +afterEach(function () { + Redis::flushdb(); +}); + +function emptyWarmStatusQueries(callable $fn): int +{ + $count = 0; + + DB::listen(function ($query) use (&$count) { + if (preg_match('/from\s+"?statuses"?/i', $query->sql)) { + $count++; + } + }); + + $fn(); + + return $count; +} + +function emptyWarmViewer(): User +{ + $viewer = User::factory()->create(); + $viewer->refresh(); + + Passport::actingAs($viewer, ['read']); + + return $viewer; +} + +it('queries for local timeline candidates once while the instance has none', function () { + emptyWarmViewer(); + + $first = emptyWarmStatusQueries(function () { + $this->getJson('/api/v1/timelines/public?limit=10')->assertOk(); + }); + + expect($first)->toBeGreaterThan(0); + + $second = emptyWarmStatusQueries(function () { + $this->getJson('/api/v1/timelines/public?limit=10')->assertOk(); + }); + + expect($second)->toBe(0); +}); + +it('queries for network timeline candidates once while the instance has none', function () { + emptyWarmViewer(); + + $first = emptyWarmStatusQueries(function () { + $this->getJson('/api/v1/timelines/public?remote=1&limit=10')->assertOk(); + }); + + expect($first)->toBeGreaterThan(0); + + $second = emptyWarmStatusQueries(function () { + $this->getJson('/api/v1/timelines/public?remote=1&limit=10')->assertOk(); + }); + + expect($second)->toBe(0); +}); + +it('records an empty warm on both instance-wide feeds', function () { + PublicTimelineService::warmCache(true, 400); + NetworkTimelineService::warmCache(true); + + expect(PublicTimelineService::shouldWarmEmpty())->toBeFalse() + ->and(NetworkTimelineService::shouldWarmEmpty())->toBeFalse(); +}); + +it('puts the warm back in play when something is written to the feed', function () { + PublicTimelineService::warmCache(true, 400); + NetworkTimelineService::warmCache(true); + + $status = Status::factory()->create([ + 'type' => 'photo', + 'scope' => 'public', + 'visibility' => 'public', + 'local' => true, + 'in_reply_to_id' => null, + 'reblog_of_id' => null, + 'uri' => null, + ]); + StatusService::get($status->id); + + PublicTimelineService::add($status->id); + NetworkTimelineService::add($status->id); + + expect(PublicTimelineService::shouldWarmEmpty())->toBeTrue() + ->and(NetworkTimelineService::shouldWarmEmpty())->toBeTrue(); +}); + +it('still clears the local timeline set when a warm finds nothing', function () { + PublicTimelineService::add(999999999); + + PublicTimelineService::warmCache(true, 400); + + expect(PublicTimelineService::count())->toBe(0); +});