fix(server): vacuum after deleting people (#18299)

* vacuum after deleting people

* update sql

server/src/queries/person.repository.sql

@@ -13,12 +13,6 @@ set
   "personId" = $1
 where
   "asset_faces"."sourceType" = $2
-VACUUM
-ANALYZE asset_faces,
-face_search,
-person
-REINDEX TABLE asset_faces
-REINDEX TABLE person
 
 -- PersonRepository.delete
 delete from "person"
@@ -29,12 +23,6 @@ where
 delete from "asset_faces"
 where
   "asset_faces"."sourceType" = $1
-VACUUM
-ANALYZE asset_faces,
-face_search,
-person
-REINDEX TABLE asset_faces
-REINDEX TABLE person
 
 -- PersonRepository.getAllWithoutFaces
 select

server/src/repositories/person.repository.ts

@@ -105,8 +105,6 @@ export class PersonRepository {
       .set({ personId: null })
       .where('asset_faces.sourceType', '=', sourceType)
       .execute();
-
-    await this.vacuum({ reindexVectors: false });
   }
 
   @GenerateSql({ params: [DummyValue.UUID] })
@@ -121,8 +119,6 @@ export class PersonRepository {
   @GenerateSql({ params: [{ sourceType: SourceType.EXIF }] })
   async deleteFaces({ sourceType }: DeleteFacesOptions): Promise<void> {
     await this.db.deleteFrom('asset_faces').where('asset_faces.sourceType', '=', sourceType).execute();
-
-    await this.vacuum({ reindexVectors: sourceType === SourceType.MACHINE_LEARNING });
   }
 
   getAllFaces(options: GetAllFacesOptions = {}) {
@@ -519,7 +515,7 @@ export class PersonRepository {
     await this.db.updateTable('asset_faces').set({ deletedAt: new Date() }).where('asset_faces.id', '=', id).execute();
   }
 
-  private async vacuum({ reindexVectors }: { reindexVectors: boolean }): Promise<void> {
+  async vacuum({ reindexVectors }: { reindexVectors: boolean }): Promise<void> {
     await sql`VACUUM ANALYZE asset_faces, face_search, person`.execute(this.db);
     await sql`REINDEX TABLE asset_faces`.execute(this.db);
     await sql`REINDEX TABLE person`.execute(this.db);

server/src/services/person.service.spec.ts

@@ -459,6 +459,7 @@ describe(PersonService.name, () => {
       await sut.handleQueueDetectFaces({ force: false });
 
       expect(mocks.assetJob.streamForDetectFacesJob).toHaveBeenCalledWith(false);
+      expect(mocks.person.vacuum).not.toHaveBeenCalled();
       expect(mocks.job.queueAll).toHaveBeenCalledWith([
         {
           name: JobName.FACE_DETECTION,
@@ -475,6 +476,7 @@ describe(PersonService.name, () => {
 
       expect(mocks.person.deleteFaces).toHaveBeenCalledWith({ sourceType: SourceType.MACHINE_LEARNING });
       expect(mocks.person.delete).toHaveBeenCalledWith([personStub.withName.id]);
+      expect(mocks.person.vacuum).toHaveBeenCalledWith({ reindexVectors: true });
       expect(mocks.storage.unlink).toHaveBeenCalledWith(personStub.withName.thumbnailPath);
       expect(mocks.assetJob.streamForDetectFacesJob).toHaveBeenCalledWith(true);
       expect(mocks.job.queueAll).toHaveBeenCalledWith([
@@ -492,6 +494,7 @@ describe(PersonService.name, () => {
 
       expect(mocks.person.delete).not.toHaveBeenCalled();
       expect(mocks.person.deleteFaces).not.toHaveBeenCalled();
+      expect(mocks.person.vacuum).not.toHaveBeenCalled();
       expect(mocks.storage.unlink).not.toHaveBeenCalled();
       expect(mocks.assetJob.streamForDetectFacesJob).toHaveBeenCalledWith(undefined);
       expect(mocks.job.queueAll).toHaveBeenCalledWith([
@@ -521,6 +524,7 @@ describe(PersonService.name, () => {
       ]);
       expect(mocks.person.delete).toHaveBeenCalledWith([personStub.randomPerson.id]);
       expect(mocks.storage.unlink).toHaveBeenCalledWith(personStub.randomPerson.thumbnailPath);
+      expect(mocks.person.vacuum).toHaveBeenCalledWith({ reindexVectors: true });
     });
   });
 
@@ -584,6 +588,7 @@ describe(PersonService.name, () => {
       expect(mocks.systemMetadata.set).toHaveBeenCalledWith(SystemMetadataKey.FACIAL_RECOGNITION_STATE, {
         lastRun: expect.any(String),
       });
+      expect(mocks.person.vacuum).not.toHaveBeenCalled();
     });
 
     it('should queue all assets', async () => {
@@ -611,6 +616,7 @@ describe(PersonService.name, () => {
       expect(mocks.systemMetadata.set).toHaveBeenCalledWith(SystemMetadataKey.FACIAL_RECOGNITION_STATE, {
         lastRun: expect.any(String),
       });
+      expect(mocks.person.vacuum).toHaveBeenCalledWith({ reindexVectors: false });
     });
 
     it('should run nightly if new face has been added since last run', async () => {
@@ -629,11 +635,14 @@ describe(PersonService.name, () => {
       mocks.person.getAllWithoutFaces.mockResolvedValue([]);
       mocks.person.unassignFaces.mockResolvedValue();
 
-      await sut.handleQueueRecognizeFaces({ force: true, nightly: true });
+      await sut.handleQueueRecognizeFaces({ force: false, nightly: true });
 
       expect(mocks.systemMetadata.get).toHaveBeenCalledWith(SystemMetadataKey.FACIAL_RECOGNITION_STATE);
       expect(mocks.person.getLatestFaceDate).toHaveBeenCalledOnce();
-      expect(mocks.person.getAllFaces).toHaveBeenCalledWith(undefined);
+      expect(mocks.person.getAllFaces).toHaveBeenCalledWith({
+        personId: null,
+        sourceType: SourceType.MACHINE_LEARNING,
+      });
       expect(mocks.job.queueAll).toHaveBeenCalledWith([
         {
           name: JobName.FACIAL_RECOGNITION,
@@ -643,6 +652,7 @@ describe(PersonService.name, () => {
       expect(mocks.systemMetadata.set).toHaveBeenCalledWith(SystemMetadataKey.FACIAL_RECOGNITION_STATE, {
         lastRun: expect.any(String),
       });
+      expect(mocks.person.vacuum).not.toHaveBeenCalled();
     });
 
     it('should skip nightly if no new face has been added since last run', async () => {
@@ -660,6 +670,7 @@ describe(PersonService.name, () => {
       expect(mocks.person.getAllFaces).not.toHaveBeenCalled();
       expect(mocks.job.queueAll).not.toHaveBeenCalled();
       expect(mocks.systemMetadata.set).not.toHaveBeenCalled();
+      expect(mocks.person.vacuum).not.toHaveBeenCalled();
     });
 
     it('should delete existing people if forced', async () => {
@@ -688,6 +699,7 @@ describe(PersonService.name, () => {
       ]);
       expect(mocks.person.delete).toHaveBeenCalledWith([personStub.randomPerson.id]);
       expect(mocks.storage.unlink).toHaveBeenCalledWith(personStub.randomPerson.thumbnailPath);
+      expect(mocks.person.vacuum).toHaveBeenCalledWith({ reindexVectors: false });
     });
   });
 

server/src/services/person.service.ts

@@ -259,6 +259,7 @@ export class PersonService extends BaseService {
     if (force) {
       await this.personRepository.deleteFaces({ sourceType: SourceType.MACHINE_LEARNING });
       await this.handlePersonCleanup();
+      await this.personRepository.vacuum({ reindexVectors: true });
     }
 
     let jobs: JobItem[] = [];
@@ -409,6 +410,7 @@ export class PersonService extends BaseService {
     if (force) {
       await this.personRepository.unassignFaces({ sourceType: SourceType.MACHINE_LEARNING });
       await this.handlePersonCleanup();
+      await this.personRepository.vacuum({ reindexVectors: false });
     } else if (waiting) {
       this.logger.debug(
         `Skipping facial recognition queueing because ${waiting} job${waiting > 1 ? 's are' : ' is'} already queued`,

server/test/repositories/person.repository.mock.ts

@@ -33,5 +33,6 @@ export const newPersonRepositoryMock = (): Mocked<RepositoryInterface<PersonRepo
     createAssetFace: vitest.fn(),
     deleteAssetFace: vitest.fn(),
     softDeleteAssetFaces: vitest.fn(),
+    vacuum: vitest.fn(),
   };
 };