Stop re-warming an instance-wide timeline that has nothing to warm

A member count of zero was the only guard on the local and network warms,
and a warm that finds no eligible posts leaves the count at zero, so a new
instance, or one whose network feed has aged out, ran the full candidate
query on every single timeline request. Both warms now record an empty
outcome and both read paths consult that record as well as the count, so
the query runs once per hour instead of once per request. Any write to
either feed puts the warm straight back in play.

The lock closures return a value as well, so a request that lost the lock
is distinguishable from one that completed a warm; both used to come back
as null.

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;
                 });
             }
 

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;
             });
         }
 

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;
         }
 

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;
         }
 

tests/Feature/Timeline/PublicNetworkEmptyWarmTest.php

@@ -0,0 +1,131 @@
+<?php
+
+use App\Models\Status;
+use App\Models\User;
+use App\Services\NetworkTimelineService;
+use App\Services\PublicTimelineService;
+use App\Services\StatusService;
+use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Redis;
+use Laravel\Passport\Passport;
+
+uses(LazilyRefreshDatabase::class);
+
+/*
+|--------------------------------------------------------------------------
+| Repeated warms of an instance-wide feed that has nothing to show
+|--------------------------------------------------------------------------
+|
+| An instance with no eligible posts yet leaves both instance-wide feeds at
+| zero members, and zero members is the only signal the read path has, so
+| every single request re-runs the full candidate query. The outcome has to
+| be recorded so the query runs once per interval, and any write to the feed
+| has to put the warm back in play.
+|
+*/
+
+beforeEach(function () {
+    config(['instance.hide_nsfw_on_public_feeds' => 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);
+});