mirror of https://github.com/pixelfed/pixelfed
Rebuild a missing home feed from the read that found it missing
The read path had three ways of answering a request whose window came back empty, and all three could leave a user staring at a blank timeline. It returned early whenever the set had any members, so a feed full of unresolvable ids never got looked at. It only attempted a rebuild for a request that sent no cursor, so a client holding a cursor got nothing on every retry. And it marked the rebuild as done before knowing whether it had worked, in a key nothing ever evicted, so a warm that threw suppressed the next five minutes of attempts, and the throw itself escaped the synchronous dispatch and turned the response into a 500. A missing cached set is now rebuilt in-request behind a non-blocking 30-second lock: a reader that loses the lock serves nothing rather than running a second full warm, one that wins releases it in a finally, and a warm that throws is logged and leaves the feed eligible for the next request. The page is then re-read through the same window helper, so the cursor the client sent survives the rebuild. The rebuild goes 100 members deep and queues the deepening job only when it filled that depth. Three existing cases asserted the old bookkeeping rather than the behaviour it was meant to protect. They keep their 200s and their empty bodies and now assert the single-warm guarantee directly.pull/7869/head
Ashley
6 days ago
parent
8b4d2cc7d9
commit
be59d31de9
@ -0,0 +1,185 @@
|
||||
<?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\Cache;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Redis;
|
||||
use Illuminate\Support\Facades\Schema;
|
||||
use Laravel\Passport\Passport;
|
||||
|
||||
uses(LazilyRefreshDatabase::class);
|
||||
|
||||
/*
|
||||
|--------------------------------------------------------------------------
|
||||
| Rebuilding a home feed while serving the read that found it missing
|
||||
|--------------------------------------------------------------------------
|
||||
|
|
||||
| A feed with no Redis key is rebuilt by the request that discovered it,
|
||||
| under a lock so concurrent readers do not all rebuild, and the page that
|
||||
| comes back has to answer the cursor the client actually sent. A rebuild
|
||||
| that fails must leave the feed eligible for the next attempt instead of
|
||||
| wedging it behind bookkeeping that says the work is already done.
|
||||
|
|
||||
*/
|
||||
|
||||
afterEach(function () {
|
||||
Redis::flushdb();
|
||||
});
|
||||
|
||||
function coldBootLockKey(int $pid): string
|
||||
{
|
||||
return 'pf:timeline:home:warm:lock:'.$pid;
|
||||
}
|
||||
|
||||
function coldBootPhoto(int $profileId, int $minutesAgo): Status
|
||||
{
|
||||
$status = Status::factory()->create([
|
||||
'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($minutesAgo),
|
||||
]);
|
||||
|
||||
StatusService::get($status->id, false);
|
||||
StatusService::getMastodon($status->id, false);
|
||||
|
||||
return $status;
|
||||
}
|
||||
|
||||
/** A viewer with a cold feed and $count eligible posts from one follow. */
|
||||
function coldBootViewerWithPosts(int $count): array
|
||||
{
|
||||
$viewer = User::factory()->create();
|
||||
$viewer->refresh();
|
||||
$author = User::factory()->create();
|
||||
$author->refresh();
|
||||
|
||||
Follower::withoutEvents(fn () => Follower::create([
|
||||
'profile_id' => $viewer->profile_id,
|
||||
'following_id' => $author->profile_id,
|
||||
]));
|
||||
|
||||
$statuses = collect(range(0, $count - 1))->map(
|
||||
fn ($i) => coldBootPhoto($author->profile_id, $count - $i)
|
||||
);
|
||||
|
||||
Redis::del(HomeTimelineService::CACHE_KEY.$viewer->profile_id);
|
||||
|
||||
return [$viewer, $author, $statuses->sortByDesc(fn ($s) => (int) $s->id)->values()];
|
||||
}
|
||||
|
||||
function coldBootStatusQueries(callable $fn): int
|
||||
{
|
||||
$count = 0;
|
||||
|
||||
DB::listen(function ($query) use (&$count) {
|
||||
if (preg_match('/from\s+"?statuses"?/i', $query->sql)) {
|
||||
$count++;
|
||||
}
|
||||
});
|
||||
|
||||
$fn();
|
||||
|
||||
return $count;
|
||||
}
|
||||
|
||||
it('honours the cursor on the page served by a cold boot', function () {
|
||||
[$viewer, , $ordered] = coldBootViewerWithPosts(12);
|
||||
|
||||
expect(HomeTimelineService::cacheKeyExists($viewer->profile_id))->toBeFalse();
|
||||
|
||||
$cursor = (int) $ordered->get(8)->id;
|
||||
|
||||
Passport::actingAs($viewer, ['read']);
|
||||
|
||||
$res = $this->getJson('/api/v1/timelines/home?limit=4&max_id='.$cursor)
|
||||
->assertOk()
|
||||
->json();
|
||||
|
||||
$ids = collect($res)->pluck('id')->map(fn ($id) => (int) $id)->all();
|
||||
|
||||
expect($ids)->not->toBeEmpty();
|
||||
|
||||
foreach ($ids as $id) {
|
||||
expect($id)->toBeLessThan($cursor);
|
||||
}
|
||||
});
|
||||
|
||||
it('does not rebuild while another request holds the warm lock', function () {
|
||||
[$viewer] = coldBootViewerWithPosts(4);
|
||||
$pid = $viewer->profile_id;
|
||||
|
||||
$held = Cache::lock(coldBootLockKey($pid), 30);
|
||||
expect($held->get())->toBeTrue();
|
||||
|
||||
Passport::actingAs($viewer, ['read']);
|
||||
|
||||
$queries = coldBootStatusQueries(function () {
|
||||
$this->getJson('/api/v1/timelines/home?limit=4')->assertOk();
|
||||
});
|
||||
|
||||
expect($queries)->toBe(0);
|
||||
|
||||
$held->release();
|
||||
});
|
||||
|
||||
it('releases the warm lock it took', function () {
|
||||
[$viewer] = coldBootViewerWithPosts(4);
|
||||
$pid = $viewer->profile_id;
|
||||
|
||||
Passport::actingAs($viewer, ['read']);
|
||||
|
||||
$this->getJson('/api/v1/timelines/home?limit=4')->assertOk();
|
||||
|
||||
$lock = Cache::lock(coldBootLockKey($pid), 30);
|
||||
|
||||
expect($lock->get())->toBeTrue();
|
||||
|
||||
$lock->release();
|
||||
});
|
||||
|
||||
it('leaves the feed rebuildable when the rebuild fails', function () {
|
||||
[$viewer, , $ordered] = coldBootViewerWithPosts(3);
|
||||
$pid = $viewer->profile_id;
|
||||
|
||||
Passport::actingAs($viewer, ['read']);
|
||||
|
||||
Schema::rename('statuses', 'statuses_unavailable');
|
||||
|
||||
try {
|
||||
$first = $this->getJson('/api/v1/timelines/home?limit=4');
|
||||
} finally {
|
||||
Schema::rename('statuses_unavailable', 'statuses');
|
||||
}
|
||||
|
||||
$first->assertOk();
|
||||
expect($first->json())->toBe([])
|
||||
->and(HomeTimelineService::shouldWarmEmpty($pid))->toBeTrue();
|
||||
|
||||
$ids = collect($this->getJson('/api/v1/timelines/home?limit=4')->assertOk()->json())
|
||||
->pluck('id')->map(fn ($id) => (int) $id)->all();
|
||||
|
||||
expect($ids)->toContain((int) $ordered->first()->id);
|
||||
});
|
||||
|
||||
it('completes a cold boot over a large candidate set promptly', function () {
|
||||
[$viewer] = coldBootViewerWithPosts(120);
|
||||
|
||||
Passport::actingAs($viewer, ['read']);
|
||||
|
||||
$start = microtime(true);
|
||||
$this->getJson('/api/v1/timelines/home?limit=20')->assertOk();
|
||||
$elapsed = (microtime(true) - $start) * 1000;
|
||||
|
||||
expect($elapsed)->toBeLessThan(3000);
|
||||
});
|
||||
Loading…
Reference in New Issue