Revert "Revert "perf: 空のタイムライン読み込み時の無駄なDBアクセスを削減するため、RedisタイムラインにダミーIDを挿入する機能を追加しました。""
This reverts commit b99ed65051.
This commit is contained in:
parent
b99ed65051
commit
33bd93ca40
|
|
@ -14,6 +14,7 @@ import type { NotesRepository } from '@/models/_.js';
|
||||||
import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
|
import { NoteEntityService } from '@/core/entities/NoteEntityService.js';
|
||||||
import { FanoutTimelineName, FanoutTimelineService } from '@/core/FanoutTimelineService.js';
|
import { FanoutTimelineName, FanoutTimelineService } from '@/core/FanoutTimelineService.js';
|
||||||
import { UtilityService } from '@/core/UtilityService.js';
|
import { UtilityService } from '@/core/UtilityService.js';
|
||||||
|
import { IdService } from '@/core/IdService.js';
|
||||||
import { isUserRelated } from '@/misc/is-user-related.js';
|
import { isUserRelated } from '@/misc/is-user-related.js';
|
||||||
import { isQuote, isRenote } from '@/misc/is-renote.js';
|
import { isQuote, isRenote } from '@/misc/is-renote.js';
|
||||||
import { CacheService } from '@/core/CacheService.js';
|
import { CacheService } from '@/core/CacheService.js';
|
||||||
|
|
@ -60,6 +61,7 @@ export class FanoutTimelineEndpointService {
|
||||||
private fanoutTimelineService: FanoutTimelineService,
|
private fanoutTimelineService: FanoutTimelineService,
|
||||||
private utilityService: UtilityService,
|
private utilityService: UtilityService,
|
||||||
private channelMutingService: ChannelMutingService,
|
private channelMutingService: ChannelMutingService,
|
||||||
|
private idService: IdService,
|
||||||
) {
|
) {
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -217,7 +219,25 @@ export class FanoutTimelineEndpointService {
|
||||||
return [...redisTimeline, ...gotFromDb];
|
return [...redisTimeline, ...gotFromDb];
|
||||||
}
|
}
|
||||||
|
|
||||||
return await ps.dbFallback(ps.untilId, ps.sinceId, ps.limit);
|
// RedisおよびDBが空の場合、次回以降の無駄なDBアクセスを防ぐためダミーIDを保存する
|
||||||
|
const gotFromDb = await ps.dbFallback(ps.untilId, ps.sinceId, ps.limit);
|
||||||
|
if (
|
||||||
|
redisResultIds.length === 0 &&
|
||||||
|
ps.sinceId == null && ps.untilId == null &&
|
||||||
|
gotFromDb.length === 0
|
||||||
|
) {
|
||||||
|
const dummyId = this.idService.gen();
|
||||||
|
|
||||||
|
Promise.all(ps.redisTimelines.map((tl, i) => {
|
||||||
|
// 有効なソースかつ結果が空だった場合のみダミーを入れる
|
||||||
|
if (redisResult[i] && redisResult[i].length === 0) {
|
||||||
|
return this.fanoutTimelineService.injectDummy(tl, dummyId);
|
||||||
|
}
|
||||||
|
return Promise.resolve();
|
||||||
|
}));
|
||||||
|
}
|
||||||
|
|
||||||
|
return gotFromDb;
|
||||||
}
|
}
|
||||||
|
|
||||||
private async getAndFilterFromDb(noteIds: string[], noteFilter: NoteFilter, idCompare: (a: string, b: string) => number): Promise<MiNote[]> {
|
private async getAndFilterFromDb(noteIds: string[], noteFilter: NoteFilter, idCompare: (a: string, b: string) => number): Promise<MiNote[]> {
|
||||||
|
|
|
||||||
|
|
@ -108,6 +108,11 @@ export class FanoutTimelineService {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@bindThis
|
||||||
|
public injectDummy(tl: FanoutTimelineName, id: string) {
|
||||||
|
return this.redisForTimelines.lpush('list:' + tl, id);
|
||||||
|
}
|
||||||
|
|
||||||
@bindThis
|
@bindThis
|
||||||
public purge(name: FanoutTimelineName) {
|
public purge(name: FanoutTimelineName) {
|
||||||
return this.redisForTimelines.del('list:' + name);
|
return this.redisForTimelines.del('list:' + name);
|
||||||
|
|
|
||||||
|
|
@ -67,6 +67,7 @@ describe('FanoutTimelineEndpointService', () => {
|
||||||
.overrideProvider(FanoutTimelineService)
|
.overrideProvider(FanoutTimelineService)
|
||||||
.useValue({
|
.useValue({
|
||||||
getMulti: jest.fn(),
|
getMulti: jest.fn(),
|
||||||
|
injectDummy: jest.fn(),
|
||||||
})
|
})
|
||||||
.compile();
|
.compile();
|
||||||
|
|
||||||
|
|
@ -119,7 +120,7 @@ describe('FanoutTimelineEndpointService', () => {
|
||||||
const dbFallback = jest.fn((_untilId: string | null, _sinceId: string | null, _limit: number) => Promise.resolve([] as MiNote[]));
|
const dbFallback = jest.fn((_untilId: string | null, _sinceId: string | null, _limit: number) => Promise.resolve([] as MiNote[]));
|
||||||
|
|
||||||
const ps = {
|
const ps = {
|
||||||
redisTimelines: ['homeTimeline', 'localTimeline'] as FanoutTimelineName[],
|
redisTimelines: [`homeTimeline:${alice.id}`, 'localTimeline'] as FanoutTimelineName[],
|
||||||
useDbFallback: true,
|
useDbFallback: true,
|
||||||
limit: 10,
|
limit: 10,
|
||||||
allowPartial: false,
|
allowPartial: false,
|
||||||
|
|
@ -130,9 +131,6 @@ describe('FanoutTimelineEndpointService', () => {
|
||||||
sinceId: null,
|
sinceId: null,
|
||||||
};
|
};
|
||||||
|
|
||||||
// See comments in original file for logic explanation.
|
|
||||||
// Essentially, we expect the fallback to start from the end of the most recent reliable timeline (HTL).
|
|
||||||
|
|
||||||
await service.getMiNotes(ps);
|
await service.getMiNotes(ps);
|
||||||
|
|
||||||
expect(dbFallback).toHaveBeenCalled();
|
expect(dbFallback).toHaveBeenCalled();
|
||||||
|
|
@ -173,7 +171,7 @@ describe('FanoutTimelineEndpointService', () => {
|
||||||
const result = await service.getMiNotes(ps);
|
const result = await service.getMiNotes(ps);
|
||||||
|
|
||||||
// With the fix, we should get note1 and note2.
|
// With the fix, we should get note1 and note2.
|
||||||
// Without the fix, we would get only note3 (or empty if limit blocked it).
|
|
||||||
expect(result).toHaveLength(2);
|
expect(result).toHaveLength(2);
|
||||||
expect(result[0].id).toBe(note1.id);
|
expect(result[0].id).toBe(note1.id);
|
||||||
expect(result[1].id).toBe(note2.id);
|
expect(result[1].id).toBe(note2.id);
|
||||||
|
|
@ -224,7 +222,7 @@ describe('FanoutTimelineEndpointService', () => {
|
||||||
const dbFallback = jest.fn((untilId: string | null, sinceId: string | null, limit: number) => Promise.resolve([] as MiNote[]));
|
const dbFallback = jest.fn((untilId: string | null, sinceId: string | null, limit: number) => Promise.resolve([] as MiNote[]));
|
||||||
|
|
||||||
const ps = {
|
const ps = {
|
||||||
redisTimelines: ['homeTimeline', 'localTimeline'] as FanoutTimelineName[],
|
redisTimelines: [`homeTimeline:${alice.id}`, 'localTimeline'] as FanoutTimelineName[],
|
||||||
useDbFallback: false,
|
useDbFallback: false,
|
||||||
limit: 10,
|
limit: 10,
|
||||||
allowPartial: true,
|
allowPartial: true,
|
||||||
|
|
@ -237,9 +235,72 @@ describe('FanoutTimelineEndpointService', () => {
|
||||||
|
|
||||||
const result = await service.getMiNotes(ps);
|
const result = await service.getMiNotes(ps);
|
||||||
|
|
||||||
// With the previous logic, note3 and note4 would be filtered out because they are older than the "threshold" (end of TL1).
|
|
||||||
// With the fixed logic (skipping filter when !useDbFallback), all notes should be present.
|
// With the fixed logic (skipping filter when !useDbFallback), all notes should be present.
|
||||||
expect(result).toHaveLength(4);
|
expect(result).toHaveLength(4);
|
||||||
expect(result.map(n => n.id)).toEqual([note1.id, note2.id, note3.id, note4.id]);
|
expect(result.map(n => n.id)).toEqual([note1.id, note2.id, note3.id, note4.id]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Test for dummy ID optimization
|
||||||
|
test('should inject dummy ID when DB fallback returns empty on initial load', async () => {
|
||||||
|
const redisResult: string[][] = [[], []]; // Empty timelines
|
||||||
|
|
||||||
|
fanoutTimelineService.getMulti.mockResolvedValue(redisResult);
|
||||||
|
|
||||||
|
// Mock dbFallback to return empty array
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||||
|
const dbFallback = jest.fn((_untilId: string | null, _sinceId: string | null, _limit: number) => Promise.resolve([] as MiNote[]));
|
||||||
|
|
||||||
|
const ps = {
|
||||||
|
redisTimelines: [`homeTimeline:${alice.id}`, 'localTimeline'] as FanoutTimelineName[],
|
||||||
|
useDbFallback: true,
|
||||||
|
limit: 10,
|
||||||
|
allowPartial: true,
|
||||||
|
excludePureRenotes: false,
|
||||||
|
dbFallback,
|
||||||
|
noteFilter: () => true,
|
||||||
|
untilId: null,
|
||||||
|
sinceId: null,
|
||||||
|
};
|
||||||
|
|
||||||
|
const result = await service.getMiNotes(ps);
|
||||||
|
|
||||||
|
expect(result).toEqual([]);
|
||||||
|
// Should have tried to inject dummy ID for both empty timelines
|
||||||
|
expect(fanoutTimelineService.injectDummy).toHaveBeenCalledTimes(2);
|
||||||
|
expect(fanoutTimelineService.injectDummy).toHaveBeenCalledWith(`homeTimeline:${alice.id}`, expect.any(String));
|
||||||
|
expect(fanoutTimelineService.injectDummy).toHaveBeenCalledWith('localTimeline', expect.any(String));
|
||||||
|
});
|
||||||
|
|
||||||
|
// Test for behavior when dummy ID exists
|
||||||
|
test('should return empty result when only dummy ID exists in Redis and DB has no newer data', async () => {
|
||||||
|
const now = Date.now();
|
||||||
|
const dummyId = idService.gen(now);
|
||||||
|
// Redis has only dummy ID
|
||||||
|
const redisResult: string[][] = [[dummyId]];
|
||||||
|
|
||||||
|
fanoutTimelineService.getMulti.mockResolvedValue(redisResult);
|
||||||
|
|
||||||
|
// Mock dbFallback (should be called to check for newer notes than the dummy ID)
|
||||||
|
|
||||||
|
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||||
|
const dbFallback = jest.fn((_untilId: string | null, _sinceId: string | null, _limit: number) => Promise.resolve([] as MiNote[]));
|
||||||
|
|
||||||
|
const ps = {
|
||||||
|
redisTimelines: [`homeTimeline:${alice.id}`] as FanoutTimelineName[],
|
||||||
|
useDbFallback: true,
|
||||||
|
limit: 10,
|
||||||
|
allowPartial: false,
|
||||||
|
excludePureRenotes: false,
|
||||||
|
dbFallback,
|
||||||
|
noteFilter: () => true,
|
||||||
|
untilId: null,
|
||||||
|
sinceId: null,
|
||||||
|
};
|
||||||
|
|
||||||
|
const result = await service.getMiNotes(ps);
|
||||||
|
|
||||||
|
expect(result).toEqual([]);
|
||||||
|
// Fallback should be called to check for newer notes (ascending check from dummy ID)
|
||||||
|
expect(dbFallback).toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue