Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(server): queue handlers shouldn't increase concurrency #2508

Merged
merged 1 commit into from May 22, 2023
Merged

chore(server): queue handlers shouldn't increase concurrency #2508

merged 1 commit into from May 22, 2023

Conversation

mertalev
Copy link
Contributor

Currently, queue handlers add an additional level of concurrency to each job despite taking up an insignificant amount of time. Once queueing is complete, this results in the remaining slower handlers of each processor to have levels of concurrency higher than intended. For instance, object tagging runs 3 jobs at a time despite object detection and image classification each being assigned a concurrency of 1, and 2 videos are transcoded at a time despite a transcode concurrency of 1.

With this PR, queue handlers will no longer offset concurrency.

@vercel
Copy link

vercel bot commented May 21, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
immich ⬜️ Ignored (Inspect) May 21, 2023 7:36pm

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you instead omit the concurrency argument? I think you can actually remove the whole object and just use a single argument for job name too

@mertalev
Copy link
Contributor Author

mertalev commented May 21, 2023

Can you instead omit the concurrency argument? I think you can actually remove the whole object and just use a single argument for job name too

Not including it seems to have the default behavior of one level of concurrency per consumer. Setting it to 0 is a workaround suggested from this issue page. (They mention assigning the total concurrency to the first handler and setting the others to 0, but from my testing the behavior is identical to just setting the queue handler concurrency to 0 instead.)

@alextran1502 alextran1502 changed the title fix(server): job concurrency chore(server): queue handlers shouldn't increase concurrency May 22, 2023
@alextran1502 alextran1502 merged commit 356f442 into immich-app:main May 22, 2023
18 checks passed
@mertalev mertalev deleted the fix/job_concurrency branch May 22, 2023 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants