Revert "perf: 空のタイムライン読み込み時の無駄なDBアクセスを削減するため、RedisタイムラインにダミーIDを挿入する機能を追加しました。"
This reverts commit 1022df5325.
This commit is contained in:
parent
1022df5325
commit
b99ed65051
|
|
@ -14,7 +14,6 @@ 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';
|
||||||
|
|
@ -61,7 +60,6 @@ export class FanoutTimelineEndpointService {
|
||||||
private fanoutTimelineService: FanoutTimelineService,
|
private fanoutTimelineService: FanoutTimelineService,
|
||||||
private utilityService: UtilityService,
|
private utilityService: UtilityService,
|
||||||
private channelMutingService: ChannelMutingService,
|
private channelMutingService: ChannelMutingService,
|
||||||
private idService: IdService,
|
|
||||||
) {
|
) {
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -219,25 +217,7 @@ export class FanoutTimelineEndpointService {
|
||||||
return [...redisTimeline, ...gotFromDb];
|
return [...redisTimeline, ...gotFromDb];
|
||||||
}
|
}
|
||||||
|
|
||||||
// RedisおよびDBが空の場合、次回以降の無駄なDBアクセスを防ぐためダミーIDを保存する
|
return await ps.dbFallback(ps.untilId, ps.sinceId, ps.limit);
|
||||||
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,11 +108,6 @@ 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,7 +67,6 @@ describe('FanoutTimelineEndpointService', () => {
|
||||||
.overrideProvider(FanoutTimelineService)
|
.overrideProvider(FanoutTimelineService)
|
||||||
.useValue({
|
.useValue({
|
||||||
getMulti: jest.fn(),
|
getMulti: jest.fn(),
|
||||||
injectDummy: jest.fn(),
|
|
||||||
})
|
})
|
||||||
.compile();
|
.compile();
|
||||||
|
|
||||||
|
|
@ -120,7 +119,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:${alice.id}`, 'localTimeline'] as FanoutTimelineName[],
|
redisTimelines: ['homeTimeline', 'localTimeline'] as FanoutTimelineName[],
|
||||||
useDbFallback: true,
|
useDbFallback: true,
|
||||||
limit: 10,
|
limit: 10,
|
||||||
allowPartial: false,
|
allowPartial: false,
|
||||||
|
|
@ -131,6 +130,9 @@ 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();
|
||||||
|
|
@ -171,7 +173,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);
|
||||||
|
|
@ -222,7 +224,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:${alice.id}`, 'localTimeline'] as FanoutTimelineName[],
|
redisTimelines: ['homeTimeline', 'localTimeline'] as FanoutTimelineName[],
|
||||||
useDbFallback: false,
|
useDbFallback: false,
|
||||||
limit: 10,
|
limit: 10,
|
||||||
allowPartial: true,
|
allowPartial: true,
|
||||||
|
|
@ -235,72 +237,9 @@ 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