From fa7dfc0264c5327d8afc08aa64ff8af36fc2fa5d Mon Sep 17 00:00:00 2001 From: Zack Pollard Date: Fri, 25 Oct 2024 12:09:58 +0100 Subject: [PATCH 1/2] fix: library scanning cron job created on every instance instead of just one --- server/src/services/library.service.spec.ts | 9 +++++++++ server/src/services/library.service.ts | 14 ++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/server/src/services/library.service.spec.ts b/server/src/services/library.service.spec.ts index 5258c8d035b52..3675b68c5309e 100644 --- a/server/src/services/library.service.spec.ts +++ b/server/src/services/library.service.spec.ts @@ -117,6 +117,15 @@ describe(LibraryService.name, () => { expect(storageMock.watch).not.toHaveBeenCalled(); }); + + it('should not initialize library scan cron job when lock is taken', async () => { + systemMock.get.mockResolvedValue(systemConfigStub.libraryWatchEnabled); + databaseMock.tryLock.mockResolvedValue(false); + + await sut.onBootstrap(); + + expect(jobMock.addCronJob).not.toHaveBeenCalled(); + }); }); describe('onConfigUpdateEvent', () => { diff --git a/server/src/services/library.service.ts b/server/src/services/library.service.ts index e319983d3b3a1..e99fd9159003b 100644 --- a/server/src/services/library.service.ts +++ b/server/src/services/library.service.ts @@ -51,12 +51,14 @@ export class LibraryService extends BaseService { this.watchLibraries = this.watchLock && watch.enabled; - this.jobRepository.addCronJob( - 'libraryScan', - scan.cronExpression, - () => handlePromiseError(this.jobRepository.queue({ name: JobName.LIBRARY_QUEUE_SYNC_ALL }), this.logger), - scan.enabled, - ); + if (this.watchLock) { + this.jobRepository.addCronJob( + 'libraryScan', + scan.cronExpression, + () => handlePromiseError(this.jobRepository.queue({ name: JobName.LIBRARY_QUEUE_SYNC_ALL }), this.logger), + scan.enabled, + ); + } if (this.watchLibraries) { await this.watchAll(); From 80f6aaf961bf0f36e2610595995f11074327adb6 Mon Sep 17 00:00:00 2001 From: Zack Pollard Date: Fri, 25 Oct 2024 14:28:59 +0100 Subject: [PATCH 2/2] fix: only run library scan and file watches on microservices --- server/src/interfaces/database.interface.ts | 2 +- server/src/services/library.service.spec.ts | 34 ++++++++++++--------- server/src/services/library.service.ts | 23 ++++++++------ 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/server/src/interfaces/database.interface.ts b/server/src/interfaces/database.interface.ts index 79550d416ea5e..da0938c8b7e1b 100644 --- a/server/src/interfaces/database.interface.ts +++ b/server/src/interfaces/database.interface.ts @@ -19,7 +19,7 @@ export enum DatabaseLock { StorageTemplateMigration = 420, VersionHistory = 500, CLIPDimSize = 512, - LibraryWatch = 1337, + Library = 1337, GetSystemConfig = 69, } diff --git a/server/src/services/library.service.spec.ts b/server/src/services/library.service.spec.ts index 3675b68c5309e..a3b270218ef48 100644 --- a/server/src/services/library.service.spec.ts +++ b/server/src/services/library.service.spec.ts @@ -3,7 +3,7 @@ import { Stats } from 'node:fs'; import { defaults, SystemConfig } from 'src/config'; import { mapLibrary } from 'src/dtos/library.dto'; import { UserEntity } from 'src/entities/user.entity'; -import { AssetType } from 'src/enum'; +import { AssetType, ImmichWorker } from 'src/enum'; import { IAssetRepository } from 'src/interfaces/asset.interface'; import { IDatabaseRepository } from 'src/interfaces/database.interface'; import { @@ -55,7 +55,7 @@ describe(LibraryService.name, () => { it('should init cron job and handle config changes', async () => { systemMock.get.mockResolvedValue(systemConfigStub.libraryScan); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); expect(jobMock.addCronJob).toHaveBeenCalled(); expect(systemMock.get).toHaveBeenCalled(); @@ -91,7 +91,7 @@ describe(LibraryService.name, () => { ), ); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); expect(storageMock.watch.mock.calls).toEqual( expect.arrayContaining([ @@ -104,7 +104,7 @@ describe(LibraryService.name, () => { it('should not initialize watcher when watching is disabled', async () => { systemMock.get.mockResolvedValue(systemConfigStub.libraryWatchDisabled); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); expect(storageMock.watch).not.toHaveBeenCalled(); }); @@ -113,7 +113,7 @@ describe(LibraryService.name, () => { systemMock.get.mockResolvedValue(systemConfigStub.libraryWatchEnabled); databaseMock.tryLock.mockResolvedValue(false); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); expect(storageMock.watch).not.toHaveBeenCalled(); }); @@ -122,7 +122,13 @@ describe(LibraryService.name, () => { systemMock.get.mockResolvedValue(systemConfigStub.libraryWatchEnabled); databaseMock.tryLock.mockResolvedValue(false); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); + + expect(jobMock.addCronJob).not.toHaveBeenCalled(); + }); + + it('should not initialize watcher or library scan job when running on api', async () => { + await sut.onBootstrap(ImmichWorker.API); expect(jobMock.addCronJob).not.toHaveBeenCalled(); }); @@ -132,7 +138,7 @@ describe(LibraryService.name, () => { beforeEach(async () => { systemMock.get.mockResolvedValue(defaults); databaseMock.tryLock.mockResolvedValue(true); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); }); it('should do nothing if oldConfig is not provided', async () => { @@ -142,7 +148,7 @@ describe(LibraryService.name, () => { it('should do nothing if instance does not have the watch lock', async () => { databaseMock.tryLock.mockResolvedValue(false); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); await sut.onConfigUpdate({ newConfig: systemConfigStub.libraryScan as SystemConfig, oldConfig: defaults }); expect(jobMock.updateCronJob).not.toHaveBeenCalled(); }); @@ -702,7 +708,7 @@ describe(LibraryService.name, () => { const mockClose = vitest.fn(); storageMock.watch.mockImplementation(makeMockWatcher({ close: mockClose })); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); await sut.delete(libraryStub.externalLibraryWithImportPaths1.id); expect(mockClose).toHaveBeenCalled(); @@ -836,7 +842,7 @@ describe(LibraryService.name, () => { libraryMock.get.mockResolvedValue(libraryStub.externalLibraryWithImportPaths1); libraryMock.getAll.mockResolvedValue([]); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); await sut.create({ ownerId: authStub.admin.user.id, importPaths: libraryStub.externalLibraryWithImportPaths1.importPaths, @@ -899,7 +905,7 @@ describe(LibraryService.name, () => { systemMock.get.mockResolvedValue(systemConfigStub.libraryWatchEnabled); libraryMock.getAll.mockResolvedValue([]); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); }); it('should throw an error if an import path is invalid', async () => { @@ -940,7 +946,7 @@ describe(LibraryService.name, () => { beforeEach(async () => { systemMock.get.mockResolvedValue(systemConfigStub.libraryWatchDisabled); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); }); it('should not watch library', async () => { @@ -956,7 +962,7 @@ describe(LibraryService.name, () => { beforeEach(async () => { systemMock.get.mockResolvedValue(systemConfigStub.libraryWatchEnabled); libraryMock.getAll.mockResolvedValue([]); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); }); it('should watch library', async () => { @@ -1122,7 +1128,7 @@ describe(LibraryService.name, () => { const mockClose = vitest.fn(); storageMock.watch.mockImplementation(makeMockWatcher({ close: mockClose })); - await sut.onBootstrap(); + await sut.onBootstrap(ImmichWorker.MICROSERVICES); await sut.onShutdown(); expect(mockClose).toHaveBeenCalledTimes(2); diff --git a/server/src/services/library.service.ts b/server/src/services/library.service.ts index e99fd9159003b..6c329e80ec140 100644 --- a/server/src/services/library.service.ts +++ b/server/src/services/library.service.ts @@ -16,7 +16,7 @@ import { } from 'src/dtos/library.dto'; import { AssetEntity } from 'src/entities/asset.entity'; import { LibraryEntity } from 'src/entities/library.entity'; -import { AssetType } from 'src/enum'; +import { AssetType, ImmichWorker } from 'src/enum'; import { DatabaseLock } from 'src/interfaces/database.interface'; import { ArgOf } from 'src/interfaces/event.interface'; import { @@ -36,22 +36,25 @@ import { validateCronExpression } from 'src/validation'; @Injectable() export class LibraryService extends BaseService { private watchLibraries = false; - private watchLock = false; + private lock = false; private watchers: Record Promise> = {}; @OnEvent({ name: 'app.bootstrap' }) - async onBootstrap() { + async onBootstrap(workerType: ImmichWorker) { + if (workerType !== ImmichWorker.MICROSERVICES) { + return; + } + const config = await this.getConfig({ withCache: false }); const { watch, scan } = config.library; // This ensures that library watching only occurs in one microservice - // TODO: we could make the lock be per-library instead of global - this.watchLock = await this.databaseRepository.tryLock(DatabaseLock.LibraryWatch); + this.lock = await this.databaseRepository.tryLock(DatabaseLock.Library); - this.watchLibraries = this.watchLock && watch.enabled; + this.watchLibraries = this.lock && watch.enabled; - if (this.watchLock) { + if (this.lock) { this.jobRepository.addCronJob( 'libraryScan', scan.cronExpression, @@ -67,7 +70,7 @@ export class LibraryService extends BaseService { @OnEvent({ name: 'config.update', server: true }) async onConfigUpdate({ newConfig: { library }, oldConfig }: ArgOf<'config.update'>) { - if (!oldConfig || !this.watchLock) { + if (!oldConfig || !this.lock) { return; } @@ -182,7 +185,7 @@ export class LibraryService extends BaseService { } private async unwatchAll() { - if (!this.watchLock) { + if (!this.lock) { return false; } @@ -192,7 +195,7 @@ export class LibraryService extends BaseService { } async watchAll() { - if (!this.watchLock) { + if (!this.lock) { return false; }