Skip to content

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

@update-docs
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
@sonarqubecloud
Copy link

sonarqubecloud 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
@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