Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(backend): DBフォールバック有効時、複数のFTTソースから取得するタイムラインで取得漏れが起きる現象の修正 #13495

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@
- Fix: カスタム絵文字の画像読み込みに失敗した際はテキストではなくダミー画像を表示 #13487

### Server
-
- Fix: FTT有効かつDBフォールバック有効時、STLのようにタイムラインのソースが複数だとFTTとDBのフォールバック間で取得されないノートがある問題

## 2024.3.0

Expand Down
15 changes: 12 additions & 3 deletions packages/backend/src/core/FanoutTimelineEndpointService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type TimelineOptions = {
excludeReplies?: boolean;
excludePureRenotes: boolean;
dbFallback: (untilId: string | null, sinceId: string | null, limit: number) => Promise<MiNote[]>,
preventEmptyTimelineDbFallback?: boolean;
};

@Injectable()
Expand Down Expand Up @@ -63,12 +64,20 @@ export class FanoutTimelineEndpointService {

const redisResult = await this.fanoutTimelineService.getMulti(ps.redisTimelines, ps.untilId, ps.sinceId);

// オプション無効時、取得したredisResultのうち、2つ以上ソースがあり、1つでも空であればDBにフォールバックする
let shouldFallbackToDb = ps.useDbFallback &&
(ps.preventEmptyTimelineDbFallback !== true && redisResult.length > 1 && redisResult.some(ids => ids.length === 0));
Comment on lines +68 to +69
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

デフォルト値がこっちのほうがわかりやすそう

Suggested change
let shouldFallbackToDb = ps.useDbFallback &&
(ps.preventEmptyTimelineDbFallback !== true && redisResult.length > 1 && redisResult.some(ids => ids.length === 0));
ps.preventEmptyTimelineDbFallback ??= false;
let shouldFallbackToDb = ps.useDbFallback &&
(!ps.preventEmptyTimelineDbFallback && redisResult.length > 1 && redisResult.some(ids => ids.length === 0));

あとはtrueのときに無効化するというのがちょっと分かりづらい気がするので

Suggested change
let shouldFallbackToDb = ps.useDbFallback &&
(ps.preventEmptyTimelineDbFallback !== true && redisResult.length > 1 && redisResult.some(ids => ids.length === 0));
ps.fallbackToDbIfHasEmptyTimeline ??= true;
let shouldFallbackToDb = ps.useDbFallback &&
(ps.fallbackToDbIfHasEmptyTimeline && redisResult.length > 1 && redisResult.some(ids => ids.length === 0));

とかのほうがいいのかもしれないと思いました

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

このPR作成時に「中身が存在しないTLがある場合はDBを参照しに行く方が取得漏れがなくなるので妥当」という意味でprevent...をtrueにするという命名を行っていたけれどtrueの時に無効化はこの命名でも、紛らわしいか…?(ちょっと上の案、下の案、あるいはやらない、ほかの案などもそれぞれ考えてみます)


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

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

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

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

if (!shouldFallbackToDb) {
let filter = ps.noteFilter ?? (_note => true);
Expand Down
1 change: 1 addition & 0 deletions packages/backend/src/server/api/endpoints/users/notes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ export default class extends Endpoint<typeof meta, typeof paramDef> { // eslint-
withFiles: ps.withFiles,
withRenotes: ps.withRenotes,
}, me),
preventEmptyTimelineDbFallback: true,
});

return timeline;
Expand Down
Loading