Keep a populated home feed when a forced warm publishes nothing

A forced warm that came up with no publishable candidates deleted the
live feed key. For a per-user feed that is destructive in exactly the
wrong direction: the warm can come up empty because the viewer's cached
following list is stale or because the statuses behind the candidates
could not be resolved, and the user who was reading their timeline a
moment ago is left with nothing. The scratch key is dropped instead and
the live feed is untouched. The instance-wide feeds keep their
clear-on-empty behaviour, which now has a comment saying why they differ.

warmCache also returns the number of members it published instead of a
constant 1, which is what lets a caller tell "rebuilt and ready" from
"nothing to show". The unused $returnIds parameter is gone; all four
call sites discarded it.

Verified standalone off staging:
HomeWarmEmptyResultTest 2 passed; HomeChronologicalOrderTest green;
tests/Feature/Timeline + HomeFeed 73 passed.
pull/7866/head
Ashley 6 days ago
parent d2e2a20705
commit 2099fa6593

@ -122,7 +122,7 @@ class RebuildHomeFeeds extends Command
}
if ($sync) {
HomeTimelineService::warmCache($pid, true, 400, true);
HomeTimelineService::warmCache($pid, true, 400);
return;
}

@ -73,6 +73,6 @@ class FeedWarmCachePipeline implements ShouldBeUniqueUntilProcessing, ShouldQueu
return;
}
HomeTimelineService::warmCache($pid, true, 400, true);
HomeTimelineService::warmCache($pid, true, 400);
}
}

@ -80,7 +80,14 @@ class HomeTimelineService
}
}
public static function warmCache($id, $force = false, $limit = 100, $returnIds = false)
/**
* Rebuild the cached feed, returning the number of members published.
*
* Returns 0 both when nothing was published and when the guard declined
* to run; the read path only ever forces, so the two cannot collide
* there, and every other caller treats them the same way.
*/
public static function warmCache($id, bool $force = false, int $limit = 100): int
{
if (self::count($id) == 0 || $force == true) {
$following = Cache::remember('profile:following:'.$id, 1209600, function () use ($id) {
@ -153,9 +160,15 @@ class HomeTimelineService
}
if ($added === 0) {
Redis::del(self::CACHE_KEY.$id);
return $returnIds ? $ids : 1;
// The instance-wide feeds clear themselves when a warm finds
// nothing, because their content is the instance's content. A
// per-user feed is different: a warm can come up empty because
// this user's relationships or the statuses behind their feed
// could not be resolved, and throwing away a feed that was
// serving content is how a user ends up with a blank timeline.
Redis::del($tmpKey);
return 0;
}
if ($added > self::FEED_LIMIT) {
@ -165,7 +178,7 @@ class HomeTimelineService
Redis::rename($tmpKey, self::CACHE_KEY.$id);
Redis::expire(self::CACHE_KEY.$id, self::CACHE_TTL);
return $returnIds ? $ids : 1;
return $added;
}
return 0;

@ -52,7 +52,7 @@ it('ranks home posts by created_at even when a remote post has a newer id but ol
StatusService::get($recent->id, false);
StatusService::get($staleRemote->id, false);
HomeTimelineService::warmCache($pid, true, 400, true);
HomeTimelineService::warmCache($pid, true, 400);
$ids = collect(HomeTimelineService::get($pid, 0, -1))->map(fn ($v) => (int) $v)->all();

@ -0,0 +1,89 @@
<?php
use App\Models\Follower;
use App\Models\Status;
use App\Models\User;
use App\Services\HomeTimelineService;
use App\Services\StatusService;
use Illuminate\Foundation\Testing\LazilyRefreshDatabase;
use Illuminate\Support\Facades\Redis;
uses(LazilyRefreshDatabase::class);
/*
|--------------------------------------------------------------------------
| What the home warm does when it finds nothing
|--------------------------------------------------------------------------
|
| A forced warm that publishes no members must not take the existing feed
| with it: a per-user feed can come up empty because the viewer's cached
| relationships or the statuses behind their feed could not be resolved,
| and throwing the live feed away is how a user who was reading a moment
| ago ends up with nothing. warmCache also returns the number of members
| it published, which is what lets a caller tell "rebuilt" from "empty".
|
*/
afterEach(function () {
Redis::flushdb();
});
function warmEmptyPhoto(int $profileId, array $overrides = []): Status
{
$status = Status::factory()->create(array_merge([
'profile_id' => $profileId,
'type' => 'photo',
'scope' => 'public',
'visibility' => 'public',
'local' => true,
'in_reply_to_id' => null,
'reblog_of_id' => null,
'uri' => null,
'created_at' => now()->subMinutes(30),
], $overrides));
StatusService::get($status->id, false);
StatusService::getMastodon($status->id, false);
return $status;
}
function warmEmptyViewerFollowing(): array
{
$viewer = User::factory()->create();
$viewer->refresh();
$author = User::factory()->create();
$author->refresh();
Follower::create([
'profile_id' => $viewer->profile_id,
'following_id' => $author->profile_id,
]);
return [$viewer, $author];
}
it('keeps a populated feed when a forced warm publishes nothing', function () {
$viewer = User::factory()->create();
$viewer->refresh();
$pid = $viewer->profile_id;
// A real status by an author the viewer does not follow: the warm's
// candidate query cannot find it, so the warm publishes nothing.
$stranger = User::factory()->create();
$stranger->refresh();
$status = warmEmptyPhoto($stranger->profile_id);
HomeTimelineService::add($pid, $status->id);
expect(HomeTimelineService::count($pid))->toBe(1);
HomeTimelineService::warmCache($pid, true, 400);
expect(HomeTimelineService::count($pid))->toBe(1)
->and(collect(HomeTimelineService::get($pid, 0, -1))->map(fn ($v) => (int) $v)->all())
->toContain((int) $status->id);
});
it('reports how many members the warm published', function () {
[$viewer, $author] = warmEmptyViewerFollowing();
$pid = $viewer->profile_id;
collect(range(1, 3))->each(fn () => warmEmptyPhoto($author->profile_id));
expect(HomeTimelineService::warmCache($pid, true, 100))->toBe(3);
$barren = User::factory()->create();
$barren->refresh();
expect(HomeTimelineService::warmCache($barren->profile_id, true, 100))->toBe(0);
});
Loading…
Cancel
Save