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

move thumbnailer ratelimiter to grpc service #10225

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented Oct 2, 2024

The THUMBNAILS_MAX_CONCURRENT_REQUESTS setting was not working correctly. Previously it was just limiting the number of concurrent thumbnail downloads. Now the limit is applied to the number thumbnail generations requests.

Related issue https://github.com/owncloud/enterprise/issues/6612#issuecomment-2389007634

Copy link

update-docs bot commented Oct 2, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@butonic
Copy link
Member

butonic commented Oct 4, 2024

related: #9240

This moves the ratelimit ('THUMBNAILS_MAX_CONCURRENT_REQUESTS') from the
HTTP endpoint to the GRPC endpoint. The HTTP endpoint is just used for
downloading already created thumbnails. But the resource consuming part of
thumbnail generation happens in the GRPC service.
@rhafer rhafer changed the title wip: move thumbnailer ratelimiter to grpc service move thumbnailer ratelimiter to grpc service Oct 8, 2024
@rhafer
Copy link
Contributor Author

rhafer commented Oct 8, 2024

related: #9240

Setting the Retry-After Header now that we are limiting on the GRPC endpoint is becoming somewhat tricky.

@rhafer rhafer marked this pull request as ready for review October 8, 2024 14:13
Copy link

sonarcloud bot commented Oct 8, 2024

@rhafer
Copy link
Contributor Author

rhafer commented Oct 9, 2024

I am still trying to figure out a good way for the Retry-After Header. But that will come with a separate PR. So from my side I think this is ready to merge.

@rhafer rhafer merged commit e2d7251 into owncloud:master Oct 9, 2024
4 checks passed
@rhafer rhafer deleted the enterprise/6612 branch October 9, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants