diff --git a/packages/backend/src/core/CacheManagementService.ts b/packages/backend/src/core/CacheManagementService.ts index 5addab4116..ab04ecbe34 100644 --- a/packages/backend/src/core/CacheManagementService.ts +++ b/packages/backend/src/core/CacheManagementService.ts @@ -16,7 +16,7 @@ import { import { QuantumKVCache, type QuantumKVOpts } from '@/misc/QuantumKVCache.js'; import { bindThis } from '@/decorators.js'; import { DI } from '@/di-symbols.js'; -import { TimeService } from '@/core/TimeService.js'; +import { TimeService, type TimerHandle } from '@/core/TimeService.js'; import { InternalEventService } from '@/core/InternalEventService.js'; // This is the one place that's *supposed* to new() up caches. @@ -28,8 +28,10 @@ export type ManagedRedisKVCache = Managed>; export type ManagedRedisSingleCache = Managed>; export type ManagedQuantumKVCache = Managed>; -export type Managed = Omit; -export type Manager = { dispose(): void, clear(): void }; +export type Managed = Omit; +export type Manager = { dispose(): void, clear(): void, gc(): void }; + +export const GC_INTERVAL = 1000 * 60 * 3; // 3m /** * Creates and "manages" instances of any standard cache type. @@ -38,6 +40,7 @@ export type Manager = { dispose(): void, clear(): void }; @Injectable() export class CacheManagementService implements OnApplicationShutdown { private readonly managedCaches = new Set(); + private gcTimer?: TimerHandle | null; constructor( @Inject(DI.redis) @@ -87,18 +90,32 @@ export class CacheManagementService implements OnApplicationShutdown { protected manageCache(cache: T): Managed { this.managedCaches.add(cache); + this.startGcTimer(); return cache; } + @bindThis + public gc(): void { + this.resetGcTimer(() => { + // TODO callAll() + for (const manager of this.managedCaches) { + manager.gc(); + } + }); + } + @bindThis public clear(): void { - for (const manager of this.managedCaches) { - manager.clear(); - } + this.resetGcTimer(() => { + for (const manager of this.managedCaches) { + manager.clear(); + } + }); } @bindThis public async dispose(): Promise { + this.stopGcTimer(); for (const manager of this.managedCaches) { manager.dispose(); } @@ -109,4 +126,32 @@ export class CacheManagementService implements OnApplicationShutdown { public async onApplicationShutdown(): Promise { await this.dispose(); } + + @bindThis + private startGcTimer() { + // Only start it once, and don't *re* start since this gets called repeatedly. + this.gcTimer ??= this.timeService.startTimer(this.gc, GC_INTERVAL, { repeated: true }); + } + + @bindThis + private stopGcTimer() { + // Only stop it once, then clear the value so it can be restarted later. + if (this.gcTimer != null) { + this.timeService.stopTimer(this.gcTimer); + this.gcTimer = null; + } + } + + @bindThis + private resetGcTimer(onBlank?: () => void): void { + this.stopGcTimer(); + + try { + if (onBlank) { + onBlank(); + } + } finally { + this.startGcTimer(); + } + } } diff --git a/packages/backend/src/misc/cache.ts b/packages/backend/src/misc/cache.ts index 098ac0cba1..0bf9ddc95d 100644 --- a/packages/backend/src/misc/cache.ts +++ b/packages/backend/src/misc/cache.ts @@ -5,10 +5,7 @@ import * as Redis from 'ioredis'; import { bindThis } from '@/decorators.js'; -import { TimeService, NativeTimeService } from '@/core/TimeService.js'; - -// This matches the default DI implementation, but as a shared instance to avoid wasting memory. -const defaultTimeService: TimeService = new NativeTimeService(); +import { TimeService } from '@/core/TimeService.js'; export interface RedisCacheServices extends MemoryCacheServices { readonly redisClient: Redis.Redis @@ -194,6 +191,11 @@ export class RedisSingleCache { return value; } + @bindThis + public gc(): void { + this.memoryCache.gc(); + } + @bindThis public async delete(): Promise { this.memoryCache.delete(); @@ -242,22 +244,20 @@ export class RedisSingleCache { } export interface MemoryCacheServices { - readonly timeService?: TimeService; + readonly timeService: TimeService; } // TODO: メモリ節約のためあまり参照されないキーを定期的に削除できるようにする? export class MemoryKVCache { private readonly cache = new Map(); - private readonly gcIntervalHandle: symbol; private readonly timeService: TimeService; constructor( private readonly lifetime: number, - services?: MemoryCacheServices, + services: MemoryCacheServices, ) { - this.timeService = services?.timeService ?? defaultTimeService; - this.gcIntervalHandle = this.timeService.startTimer(() => this.gc(), 1000 * 60 * 3, { repeated: true }); // 3m + this.timeService = services.timeService; } @bindThis @@ -375,7 +375,6 @@ export class MemoryKVCache { @bindThis public dispose(): void { this.clear(); - this.timeService.stopTimer(this.gcIntervalHandle); } public get size() { @@ -394,9 +393,9 @@ export class MemorySingleCache { constructor( private lifetime: number, - services?: MemoryCacheServices, + services: MemoryCacheServices, ) { - this.timeService = services?.timeService ?? defaultTimeService; + this.timeService = services.timeService; } @bindThis @@ -406,13 +405,24 @@ export class MemorySingleCache { } @bindThis - public get(): T | undefined { - if (this.cachedAt == null) return undefined; - if ((this.timeService.now - this.cachedAt) > this.lifetime) { - this.value = undefined; - this.cachedAt = null; - return undefined; + public gc(): void { + // Check if we have a valid, non-expired value. + // This is a little convoluted but protects against edge cases and invalid states. + if (this.value !== undefined && this.cachedAt != null) { + const age = this.timeService.now - this.cachedAt; + if (Number.isSafeInteger(age) && age <= this.lifetime) { + return; + } } + + // If we get here, then it's expired or otherwise invalid. + // Whatever the case, we should clear everything back to zeros. + this.delete(); + } + + @bindThis + public get(): T | undefined { + this.gc(); return this.value; } diff --git a/packages/backend/test/unit/core/CacheManagementService.ts b/packages/backend/test/unit/core/CacheManagementService.ts index 477e45fa8a..d60d7a8d2d 100644 --- a/packages/backend/test/unit/core/CacheManagementService.ts +++ b/packages/backend/test/unit/core/CacheManagementService.ts @@ -6,44 +6,66 @@ import { jest } from '@jest/globals'; import { MockRedis } from '../../misc/MockRedis.js'; import { GodOfTimeService } from '../../misc/GodOfTimeService.js'; -import { CacheManagementService, type Manager } from '@/core/CacheManagementService.js'; -import { InternalEventService } from '@/core/InternalEventService.js'; +import { MockInternalEventService } from '../../misc/MockInternalEventService.js'; +import { CacheManagementService, type Manager, GC_INTERVAL } from '@/core/CacheManagementService.js'; import { MemoryKVCache } from '@/misc/cache.js'; describe(CacheManagementService, () => { let timeService: GodOfTimeService; let redisClient: MockRedis; - let internalEventService: InternalEventService; + let internalEventService: MockInternalEventService; let serviceUnderTest: CacheManagementService; let internalsUnderTest: { managedCaches: Set }; - beforeEach(() => { + beforeAll(() => { timeService = new GodOfTimeService(); - timeService.resetToNow(); redisClient = new MockRedis(timeService); - internalEventService = new InternalEventService(redisClient, redisClient, { host: 'example.com' }); + internalEventService = new MockInternalEventService( { host: 'example.com' }); + }); + + afterAll(() => { + internalEventService.dispose(); + redisClient.disconnect(); + }); + + beforeEach(() => { + timeService.resetToNow(); + redisClient.mockReset(); + internalEventService.mockReset(); serviceUnderTest = new CacheManagementService(redisClient, timeService, internalEventService); internalsUnderTest = { managedCaches: Reflect.get(serviceUnderTest, 'managedCaches') }; }); afterEach(() => { - timeService.reset(); serviceUnderTest.dispose(); - internalEventService.dispose(); - redisClient.disconnect(); }); + function createCache(): MemoryKVCache { + // Cast to allow access to managed functions, for spying purposes. + return serviceUnderTest.createMemoryKVCache(Infinity) as MemoryKVCache; + } + describe('createMemoryKVCache', () => testCreate('createMemoryKVCache', Infinity)); describe('createMemorySingleCache', () => testCreate('createMemorySingleCache', Infinity)); describe('createRedisKVCache', () => testCreate('createRedisKVCache', 'redis', { lifetime: Infinity, memoryCacheLifetime: Infinity })); describe('createRedisSingleCache', () => testCreate('createRedisSingleCache', 'single', { lifetime: Infinity, memoryCacheLifetime: Infinity })); describe('createQuantumKVCache', () => testCreate('createQuantumKVCache', 'quantum', { lifetime: Infinity, fetcher: () => { throw new Error('not implement'); } })); - describe('clear', () => testClear('clear', false)); - describe('dispose', () => testClear('dispose', true)); - describe('onApplicationShutdown', () => testClear('onApplicationShutdown', true)); + describe('clear', () => { + testClear('clear', false); + testGC('clear', false, true, false); + }); + describe('dispose', () => { + testClear('dispose', true); + testGC('dispose', false, false, true); + }); + describe('onApplicationShutdown', () => { + testClear('onApplicationShutdown', true); + testGC('onApplicationShutdown', false, false, true); + }); + describe('gc', () => testGC('gc', true, true, false)); function testCreate(func: Func, ...args: Parameters) { // @ts-expect-error TypeScript bug: https://github.com/microsoft/TypeScript/issues/57322 @@ -60,15 +82,22 @@ describe(CacheManagementService, () => { expect(internalsUnderTest.managedCaches).toContain(cache); }); + + it('should start GC timer', () => { + const cache = act(); + const gc = jest.spyOn(cache as unknown as { gc(): void }, 'gc'); + + timeService.tick({ milliseconds: GC_INTERVAL * 3 }); + + expect(gc).toHaveBeenCalledTimes(3); + }); } function testClear(func: 'clear' | 'dispose' | 'onApplicationShutdown', shouldDispose: boolean) { - function act() { - serviceUnderTest[func](); - } + const act = () => serviceUnderTest[func](); it('should clear managed caches', () => { - const cache = serviceUnderTest.createMemoryKVCache(Infinity); + const cache = createCache(); const clear = jest.spyOn(cache, 'clear'); act(); @@ -77,8 +106,8 @@ describe(CacheManagementService, () => { }); it(`should${shouldDispose ? ' ' : ' not '}dispose managed caches`, () => { - const cache = serviceUnderTest.createMemoryKVCache(Infinity); - const dispose = jest.spyOn(cache as MemoryKVCache, 'dispose'); + const cache = createCache(); + const dispose = jest.spyOn(cache, 'dispose'); act(); @@ -94,7 +123,7 @@ describe(CacheManagementService, () => { }); it('should be callable multiple times', () => { - const cache = serviceUnderTest.createMemoryKVCache(Infinity); + const cache = createCache(); const clear = jest.spyOn(cache, 'clear'); act(); @@ -106,7 +135,7 @@ describe(CacheManagementService, () => { }); it(`should${shouldDispose ? ' ' : ' not '}deref caches`, () => { - const cache = serviceUnderTest.createMemoryKVCache(Infinity); + const cache = createCache(); act(); @@ -118,7 +147,7 @@ describe(CacheManagementService, () => { }); it(`should${shouldDispose ? ' ' : ' not '}reset cache list`, () => { - serviceUnderTest.createMemoryKVCache(Infinity); + createCache(); act(); @@ -129,4 +158,43 @@ describe(CacheManagementService, () => { } }); } + + function testGC(func: 'clear' | 'dispose' | 'onApplicationShutdown' | 'gc', shouldFire: boolean, shouldReset: boolean, shouldStop: boolean) { + const expectedCalls = + shouldStop + ? shouldFire + ? 1 + : 0 + : shouldFire + ? shouldReset + ? 2 + : 3 + : shouldReset + ? 1 + : 2 + ; + + const testName = 'should ' + [ + shouldFire ? 'trigger' : 'not trigger', + shouldReset ? 'reset' : 'not reset', + shouldStop ? 'and stop' : 'and not stop', + ].join(', ') + ' GC'; + + const arrange = () => jest.spyOn(createCache(), 'gc'); + const act = () => { + timeService.tick({ milliseconds: GC_INTERVAL - 1 }); + serviceUnderTest[func](); + timeService.tick({ milliseconds: 1 }); + timeService.tick({ milliseconds: GC_INTERVAL }); + }; + const assert = (spy: ReturnType) => { + expect(spy).toHaveBeenCalledTimes(expectedCalls); + }; + + it(testName, () => { + const spy = arrange(); + act(); + assert(spy); + }); + } });