cjones/immich
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(server): Enqueue jobs in bulk (#5974)

Browse Source 
* feat(server): Enqueue jobs in bulk

The Job Repository now has a `queueAll` method, that enqueues messages
in bulk (using BullMQ's
[`addBulk`](https://docs.bullmq.io/guide/queues/adding-bulks)),
improving performance when many jobs must be enqueued within the same
operation.

Primary change is in `src/domain/job/job.service.ts`, and other services
have been refactored to use `queueAll` when useful.

As a simple local benchmark, triggering a full thumbnail generation
process over a library of ~1,200 assets and ~350 faces went from
**~600ms** to **~250ms**.

* fix: Review feedback
This commit is contained in:
Michael Manganiello
2024-01-01 15:45:42 -05:00
committed by GitHub
parent 7dd88c4114
commit 4a5b8c3770
20 changed files with 323 additions and 227 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/domain/person/person.service.spec.ts
+39 -21
View File
@@ -286,6 +286,7 @@ describe(PersonService.name, () => {
expect(personMock.getById).toHaveBeenCalledWith('person-1');
expect(personMock.update).toHaveBeenCalledWith({ id: 'person-1', birthDate: new Date('1976-06-30') });
expect(jobMock.queue).not.toHaveBeenCalled();
expect(jobMock.queueAll).not.toHaveBeenCalled();
expect(accessMock.person.checkOwnerAccess).toHaveBeenCalledWith(authStub.admin.user.id, new Set(['person-1']));
});
@@ -403,6 +404,7 @@ describe(PersonService.name, () => {
}),
).rejects.toBeInstanceOf(BadRequestException);
expect(jobMock.queue).not.toHaveBeenCalledWith();
expect(jobMock.queueAll).not.toHaveBeenCalledWith();
});
it('should reassign a face', async () => {
accessMock.person.checkOwnerAccess.mockResolvedValue(new Set([personStub.withName.id]));
@@ -417,10 +419,12 @@ describe(PersonService.name, () => {
}),
).resolves.toEqual([personStub.noName]);
expect(jobMock.queue).toHaveBeenCalledWith({
name: JobName.GENERATE_PERSON_THUMBNAIL,
data: { id: personStub.newThumbnail.id },
});
expect(jobMock.queueAll).toHaveBeenCalledWith([
{
name: JobName.GENERATE_PERSON_THUMBNAIL,
data: { id: personStub.newThumbnail.id },
},
]);
});
});
@@ -452,10 +456,12 @@ describe(PersonService.name, () => {
it('should change person feature photo', async () => {
personMock.getRandomFace.mockResolvedValue(faceStub.primaryFace1);
await sut.createNewFeaturePhoto([personStub.newThumbnail.id]);
expect(jobMock.queue).toHaveBeenCalledWith({
name: JobName.GENERATE_PERSON_THUMBNAIL,
data: { id: personStub.newThumbnail.id },
});
expect(jobMock.queueAll).toHaveBeenCalledWith([
{
name: JobName.GENERATE_PERSON_THUMBNAIL,
data: { id: personStub.newThumbnail.id },
},
]);
});
});
@@ -480,6 +486,7 @@ describe(PersonService.name, () => {
});
expect(jobMock.queue).not.toHaveBeenCalledWith();
expect(jobMock.queueAll).not.toHaveBeenCalledWith();
});
it('should fail if user has not the correct permissions on the asset', async () => {
@@ -495,6 +502,7 @@ describe(PersonService.name, () => {
).rejects.toBeInstanceOf(BadRequestException);
expect(jobMock.queue).not.toHaveBeenCalledWith();
expect(jobMock.queueAll).not.toHaveBeenCalledWith();
});
});
@@ -542,7 +550,9 @@ describe(PersonService.name, () => {
await sut.handlePersonCleanup();
expect(jobMock.queue).toHaveBeenCalledWith({ name: JobName.PERSON_DELETE, data: { id: personStub.noName.id } });
expect(jobMock.queueAll).toHaveBeenCalledWith([
{ name: JobName.PERSON_DELETE, data: { id: personStub.noName.id } },
]);
});
});
@@ -552,6 +562,7 @@ describe(PersonService.name, () => {
await expect(sut.handleQueueRecognizeFaces({})).resolves.toBe(true);
expect(jobMock.queue).not.toHaveBeenCalled();
expect(jobMock.queueAll).not.toHaveBeenCalled();
expect(configMock.load).toHaveBeenCalled();
});
@@ -563,10 +574,12 @@ describe(PersonService.name, () => {
await sut.handleQueueRecognizeFaces({});
expect(assetMock.getWithout).toHaveBeenCalledWith({ skip: 0, take: 1000 }, WithoutProperty.FACES);
expect(jobMock.queue).toHaveBeenCalledWith({
name: JobName.RECOGNIZE_FACES,
data: { id: assetStub.image.id },
});
expect(jobMock.queueAll).toHaveBeenCalledWith([
{
name: JobName.RECOGNIZE_FACES,
data: { id: assetStub.image.id },
},
]);
});
it('should queue all assets', async () => {
@@ -580,14 +593,18 @@ describe(PersonService.name, () => {
await sut.handleQueueRecognizeFaces({ force: true });
expect(assetMock.getAll).toHaveBeenCalled();
expect(jobMock.queue).toHaveBeenCalledWith({
name: JobName.RECOGNIZE_FACES,
data: { id: assetStub.image.id },
});
expect(jobMock.queue).toHaveBeenCalledWith({
name: JobName.PERSON_DELETE,
data: { id: personStub.withName.id },
});
expect(jobMock.queueAll).toHaveBeenCalledWith([
{
name: JobName.RECOGNIZE_FACES,
data: { id: assetStub.image.id },
},
]);
expect(jobMock.queueAll).toHaveBeenCalledWith([
{
name: JobName.PERSON_DELETE,
data: { id: personStub.withName.id },
},
]);
});
});
@@ -644,6 +661,7 @@ describe(PersonService.name, () => {
);
expect(personMock.createFace).not.toHaveBeenCalled();
expect(jobMock.queue).not.toHaveBeenCalled();
expect(jobMock.queueAll).not.toHaveBeenCalled();
expect(assetMock.upsertJobStatus).toHaveBeenCalledWith({
assetId: assetStub.image.id,
server/src/domain/person/person.service.ts
+15 -14
View File
@@ -22,6 +22,7 @@ import {
ISmartInfoRepository,
IStorageRepository,
ISystemConfigRepository,
JobItem,
UpdateFacesData,
WithoutProperty,
} from '../repositories';
@@ -153,6 +154,8 @@ export class PersonService {
this.logger.debug(
`Changing feature photos for ${changeFeaturePhoto.length} ${changeFeaturePhoto.length > 1 ? 'people' : 'person'}`,
);
const jobs: JobItem[] = [];
for (const personId of changeFeaturePhoto) {
const assetFace = await this.repository.getRandomFace(personId);
@@ -161,15 +164,11 @@ export class PersonService {
id: personId,
faceAssetId: assetFace.id,
});
await this.jobRepository.queue({
name: JobName.GENERATE_PERSON_THUMBNAIL,
data: {
id: personId,
},
});
jobs.push({ name: JobName.GENERATE_PERSON_THUMBNAIL, data: { id: personId } });
}
}
await this.jobRepository.queueAll(jobs);
}
async getById(auth: AuthDto, id: string): Promise<PersonResponseDto> {
@@ -270,8 +269,10 @@ export class PersonService {
const people = await this.repository.getAllWithoutFaces();
for (const person of people) {
this.logger.debug(`Person ${person.name || person.id} no longer has any faces, deleting.`);
await this.jobRepository.queue({ name: JobName.PERSON_DELETE, data: { id: person.id } });
}
await this.jobRepository.queueAll(
people.map((person) => ({ name: JobName.PERSON_DELETE, data: { id: person.id } })),
);
return true;
}
@@ -290,16 +291,16 @@ export class PersonService {
if (force) {
const people = await this.repository.getAll();
for (const person of people) {
await this.jobRepository.queue({ name: JobName.PERSON_DELETE, data: { id: person.id } });
}
await this.jobRepository.queueAll(
people.map((person) => ({ name: JobName.PERSON_DELETE, data: { id: person.id } })),
);
this.logger.debug(`Deleted ${people.length} people`);
}
for await (const assets of assetPagination) {
for (const asset of assets) {
await this.jobRepository.queue({ name: JobName.RECOGNIZE_FACES, data: { id: asset.id } });
}
await this.jobRepository.queueAll(
assets.map((asset) => ({ name: JobName.RECOGNIZE_FACES, data: { id: asset.id } })),
);
}
return true;