From f7d85979474f343d7a83426cd86bc78852007471 Mon Sep 17 00:00:00 2001 From: mertalev <101130780+mertalev@users.noreply.github.com> Date: Fri, 10 May 2024 13:10:58 -0400 Subject: [PATCH 1/2] cache `getConfig` --- server/src/cores/system-config.core.ts | 110 +++++++++++-------- server/src/interfaces/database.interface.ts | 1 + server/src/services/metadata.service.spec.ts | 15 ++- 3 files changed, 73 insertions(+), 53 deletions(-) diff --git a/server/src/cores/system-config.core.ts b/server/src/cores/system-config.core.ts index 17d9ed2270871..c94da0977bbbf 100644 --- a/server/src/cores/system-config.core.ts +++ b/server/src/cores/system-config.core.ts @@ -1,5 +1,6 @@ import { BadRequestException, ForbiddenException, Injectable } from '@nestjs/common'; import { CronExpression } from '@nestjs/schedule'; +import AsyncLock from 'async-lock'; import { plainToInstance } from 'class-transformer'; import { validate } from 'class-validator'; import { load as loadYaml } from 'js-yaml'; @@ -21,6 +22,7 @@ import { TranscodePolicy, VideoCodec, } from 'src/entities/system-config.entity'; +import { DatabaseLock } from 'src/interfaces/database.interface'; import { QueueName } from 'src/interfaces/job.interface'; import { ILoggerRepository } from 'src/interfaces/logger.interface'; import { ISystemConfigRepository } from 'src/interfaces/system-config.interface'; @@ -186,7 +188,9 @@ let instance: SystemConfigCore | null; @Injectable() export class SystemConfigCore { - private configCache: SystemConfigEntity[] | null = null; + private readonly asyncLock = new AsyncLock(); + private config: SystemConfig | null = null; + private lastUpdated: number | null = null; public config$ = new Subject(); @@ -268,32 +272,17 @@ export class SystemConfigCore { } public async getConfig(force = false): Promise { - const configFilePath = process.env.IMMICH_CONFIG_FILE; - const config = _.cloneDeep(defaults); - const overrides = configFilePath ? await this.loadFromFile(configFilePath, force) : await this.repository.load(); - - for (const { key, value } of overrides) { - // set via dot notation - _.set(config, key, value); - } - - const errors = await validate(plainToInstance(SystemConfigDto, config)); - if (errors.length > 0) { - this.logger.error('Validation error', errors); - if (configFilePath) { - throw new Error(`Invalid value(s) in file: ${errors}`); - } - } - - if (!config.ffmpeg.acceptedVideoCodecs.includes(config.ffmpeg.targetVideoCodec)) { - config.ffmpeg.acceptedVideoCodecs.unshift(config.ffmpeg.targetVideoCodec); - } - - if (!config.ffmpeg.acceptedAudioCodecs.includes(config.ffmpeg.targetAudioCodec)) { - config.ffmpeg.acceptedAudioCodecs.unshift(config.ffmpeg.targetAudioCodec); + if (force || !this.config) { + const lastUpdated = this.lastUpdated; + await this.asyncLock.acquire(DatabaseLock[DatabaseLock.GetSystemConfig], async () => { + if (lastUpdated === this.lastUpdated) { + this.config = await this.buildConfig(); + this.lastUpdated = Date.now(); + } + }); } - return config; + return this.config!; } public async updateConfig(newConfig: SystemConfig): Promise { @@ -345,33 +334,60 @@ export class SystemConfigCore { this.config$.next(newConfig); } - private async loadFromFile(filepath: string, force = false) { - if (force || !this.configCache) { - try { - const file = await this.repository.readFile(filepath); - const config = loadYaml(file.toString()) as any; - const overrides: SystemConfigEntity[] = []; - - for (const key of Object.values(SystemConfigKey)) { - const value = _.get(config, key); - this.unsetDeep(config, key); - if (value !== undefined) { - overrides.push({ key, value }); - } - } + private async buildConfig() { + const config = _.cloneDeep(defaults); + const overrides = process.env.IMMICH_CONFIG_FILE + ? await this.loadFromFile(process.env.IMMICH_CONFIG_FILE) + : await this.repository.load(); - if (!_.isEmpty(config)) { - this.logger.warn(`Unknown keys found: ${JSON.stringify(config, null, 2)}`); - } + for (const { key, value } of overrides) { + // set via dot notation + _.set(config, key, value); + } - this.configCache = overrides; - } catch (error: Error | any) { - this.logger.error(`Unable to load configuration file: ${filepath}`); - throw error; + const errors = await validate(plainToInstance(SystemConfigDto, config)); + if (errors.length > 0) { + if (process.env.IMMICH_CONFIG_FILE) { + throw new Error(`Invalid value(s) in file: ${errors}`); + } else { + this.logger.error('Validation error', errors); } } - return this.configCache; + if (!config.ffmpeg.acceptedVideoCodecs.includes(config.ffmpeg.targetVideoCodec)) { + config.ffmpeg.acceptedVideoCodecs.push(config.ffmpeg.targetVideoCodec); + } + + if (!config.ffmpeg.acceptedAudioCodecs.includes(config.ffmpeg.targetAudioCodec)) { + config.ffmpeg.acceptedAudioCodecs.push(config.ffmpeg.targetAudioCodec); + } + + return config; + } + + private async loadFromFile(filepath: string) { + try { + const file = await this.repository.readFile(filepath); + const config = loadYaml(file.toString()) as any; + const overrides: SystemConfigEntity[] = []; + + for (const key of Object.values(SystemConfigKey)) { + const value = _.get(config, key); + this.unsetDeep(config, key); + if (value !== undefined) { + overrides.push({ key, value }); + } + } + + if (!_.isEmpty(config)) { + this.logger.warn(`Unknown keys found: ${JSON.stringify(config, null, 2)}`); + } + + return overrides; + } catch (error: Error | any) { + this.logger.error(`Unable to load configuration file: ${filepath}`); + throw error; + } } private unsetDeep(object: object, key: string) { diff --git a/server/src/interfaces/database.interface.ts b/server/src/interfaces/database.interface.ts index 42342eccc3cd3..9bff555b03f2c 100644 --- a/server/src/interfaces/database.interface.ts +++ b/server/src/interfaces/database.interface.ts @@ -20,6 +20,7 @@ export enum DatabaseLock { StorageTemplateMigration = 420, CLIPDimSize = 512, LibraryWatch = 1337, + GetSystemConfig = 304, } export const extName: Record = { diff --git a/server/src/services/metadata.service.spec.ts b/server/src/services/metadata.service.spec.ts index 5adeb1c7748fc..c0758eeeaf5a4 100644 --- a/server/src/services/metadata.service.spec.ts +++ b/server/src/services/metadata.service.spec.ts @@ -99,19 +99,22 @@ describe(MetadataService.name, () => { }); describe('init', () => { - beforeEach(async () => { - configMock.load.mockResolvedValue([{ key: SystemConfigKey.REVERSE_GEOCODING_ENABLED, value: true }]); - + it('should pause and resume queue during init', async () => { await sut.init(); + + expect(jobMock.pause).toHaveBeenCalledTimes(1); + expect(metadataMock.init).toHaveBeenCalledTimes(1); + expect(jobMock.resume).toHaveBeenCalledTimes(1); }); it('should return if reverse geocoding is disabled', async () => { configMock.load.mockResolvedValue([{ key: SystemConfigKey.REVERSE_GEOCODING_ENABLED, value: false }]); await sut.init(); - expect(jobMock.pause).toHaveBeenCalledTimes(1); - expect(metadataMock.init).toHaveBeenCalledTimes(1); - expect(jobMock.resume).toHaveBeenCalledTimes(1); + + expect(jobMock.pause).not.toHaveBeenCalled(); + expect(metadataMock.init).not.toHaveBeenCalled(); + expect(jobMock.resume).not.toHaveBeenCalled(); }); }); From f68d5bc6a3d39d04bf437202117ec77196bb6e57 Mon Sep 17 00:00:00 2001 From: Mert <101130780+mertalev@users.noreply.github.com> Date: Fri, 10 May 2024 14:10:09 -0400 Subject: [PATCH 2/2] critical fix Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com> --- server/src/interfaces/database.interface.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/interfaces/database.interface.ts b/server/src/interfaces/database.interface.ts index 9bff555b03f2c..e57c91e917111 100644 --- a/server/src/interfaces/database.interface.ts +++ b/server/src/interfaces/database.interface.ts @@ -20,7 +20,7 @@ export enum DatabaseLock { StorageTemplateMigration = 420, CLIPDimSize = 512, LibraryWatch = 1337, - GetSystemConfig = 304, + GetSystemConfig = 69, } export const extName: Record = {