Skip to content

Commit

Permalink
fix(backend): fallback if sinceId is older than the oldest in cache…
Browse files Browse the repository at this point in the history
… when using FTT (misskey-dev#14061)

* fix(backend): fallback if `sinceId` is older than the oldest in cache when using FTT

* Update CHANGELOG.md

* chore: fix description of test
  • Loading branch information
zyoshoka authored and Sayamame-beans committed Jul 7, 2024
1 parent d475e7b commit 77faa26
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
### Client

### Server
- Fix: FTT有効時、タイムライン用エンドポイントで`sinceId`にキャッシュ内最古のものより古いものを指定した場合に正しく結果が返ってこない問題を修正

## 2024.5.0-kinel.3

Expand Down
13 changes: 5 additions & 8 deletions packages/backend/src/core/FanoutTimelineEndpointService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ export class FanoutTimelineEndpointService {

@bindThis
private async getMiNotes(ps: TimelineOptions): Promise<MiNote[]> {
let noteIds: string[];
let shouldFallbackToDb = false;

// 呼び出し元と以下の処理をシンプルにするためにdbFallbackを置き換える
if (!ps.useDbFallback) ps.dbFallback = () => Promise.resolve([]);

Expand All @@ -67,18 +64,18 @@ export class FanoutTimelineEndpointService {
const redisResult = await this.fanoutTimelineService.getMulti(ps.redisTimelines, ps.untilId, ps.sinceId);

// 取得したredisResultのうち、2つ以上ソースがあり、1つでも空であればDBにフォールバックする
shouldFallbackToDb = ps.useDbFallback && (redisResult.length > 1 && redisResult.some(ids => ids.length === 0));
let shouldFallbackToDb = ps.useDbFallback && (redisResult.length > 1 && redisResult.some(ids => ids.length === 0));

// 取得したresultの中で最古のIDのうち、最も新しいものを取得
const thresholdId = redisResult.map(ids => ids[0]).sort()[0];

// TODO: いい感じにgetMulti内でソート済だからuniqするときにredisResultが全てソート済なのを利用して再ソートを避けたい
const redisResultIds = shouldFallbackToDb ? [] : Array.from(new Set(redisResult.flat(1)));
const redisResultIds = shouldFallbackToDb ? [] : Array.from(new Set(redisResult.flat(1))).sort(idCompare);

redisResultIds.sort(idCompare);
noteIds = redisResultIds.filter(id => id >= thresholdId).slice(0, ps.limit);
let noteIds = redisResultIds.filter(id => id >= thresholdId).slice(0, ps.limit);

shouldFallbackToDb = shouldFallbackToDb || (noteIds.length === 0);
const oldestNoteId = ascending ? redisResultIds[0] : redisResultIds[redisResultIds.length - 1];
shouldFallbackToDb = shouldFallbackToDb || (noteIds.length === 0) || (ps.sinceId != null && ps.sinceId < oldestNoteId);

if (!shouldFallbackToDb) {
let filter = ps.noteFilter ?? (_note => true);
Expand Down
35 changes: 35 additions & 0 deletions packages/backend/test/e2e/timelines.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
// pnpm jest -- e2e/timelines.ts

import * as assert from 'assert';
import { Redis } from 'ioredis';
import { loadConfig } from '@/config.js';
import { api, post, randomString, sendEnvUpdateRequest, signup, sleep, uploadUrl } from '../utils.js';

function genHost() {
Expand All @@ -17,7 +19,13 @@ function waitForPushToTl() {
return sleep(500);
}

let redisForTimelines: Redis;

describe('Timelines', () => {
beforeAll(() => {
redisForTimelines = new Redis(loadConfig().redisForTimelines);
});

describe('Home TL', () => {
test.concurrent('自分の visibility: followers なノートが含まれる', async () => {
const [alice] = await Promise.all([signup()]);
Expand Down Expand Up @@ -1373,6 +1381,33 @@ describe('Timelines', () => {

assert.strictEqual(res.body.some((note: any) => note.id === bobNote.id), false);
});

/** @see https://github.com/misskey-dev/misskey/issues/14000 */
test.concurrent('FTT: sinceId にキャッシュより古いノートを指定しても、sinceId による絞り込みが正しく動作する', async () => {
const alice = await signup();
const noteSince = await post(alice, { text: 'Note where id will be `sinceId`.' });
const note1 = await post(alice, { text: '1' });
const note2 = await post(alice, { text: '2' });
await redisForTimelines.del('list:userTimeline:' + alice.id);
const note3 = await post(alice, { text: '3' });

const res = await api('users/notes', { userId: alice.id, sinceId: noteSince.id });
assert.deepStrictEqual(res.body, [note1, note2, note3]);
});

test.concurrent('FTT: sinceId にキャッシュより古いノートを指定しても、sinceId と untilId による絞り込みが正しく動作する', async () => {
const alice = await signup();
const noteSince = await post(alice, { text: 'Note where id will be `sinceId`.' });
const note1 = await post(alice, { text: '1' });
const note2 = await post(alice, { text: '2' });
await redisForTimelines.del('list:userTimeline:' + alice.id);
const note3 = await post(alice, { text: '3' });
const noteUntil = await post(alice, { text: 'Note where id will be `untilId`.' });
await post(alice, { text: '4' });

const res = await api('users/notes', { userId: alice.id, sinceId: noteSince.id, untilId: noteUntil.id });
assert.deepStrictEqual(res.body, [note3, note2, note1]);
});
});

// TODO: リノートミュート済みユーザーのテスト
Expand Down

0 comments on commit 77faa26

Please sign in to comment.