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

Nginx: add rate limiting #9904

Conversation

scottbarnes
Copy link
Collaborator

Closes #9562

Feature. Adds rate limiting. See https://blog.nginx.org/blog/rate-limiting-nginx and https://nginx.org/en/docs/http/ngx_http_limit_req_module.html

Technical

We may wish to rely on 429 and 403 as distinct status codes rather than commingle them in 444.

We may also wish to revisit whether blocking CloudFront could actually increase load on our servers, given as a CDN it may be closer to some patrons.

Additionally, and probably as distinct from this PR, we may wish to consider drastically increasing the cache-control value for cover images, as it's currently 10800 seconds, or 3 hours, when it could probably be a year, on the perhaps mistaken theory cover images don't change. Though perhaps that would just fill up patron's caches, unless we conditionally set the cache-control value for CDNs. Anyway, another issue.

Testing

Make a lot of requests and observe the responses (it can take a while to finally start getting rate limited): seq 2000 | xargs -n 1 -P 100 bash -c 'curl https://openlibrary.org/api/books.json\?bibkeys\=ISBN:9781094376196\&jscmd\=data\&format\=json'

Screenshot

Stakeholders

@mekarpeles
@cdrini

@scottbarnes scottbarnes marked this pull request as draft September 24, 2024 02:32
@github-actions github-actions bot added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label Sep 24, 2024
docker/web_nginx.conf Outdated Show resolved Hide resolved
docker/web_nginx.conf Outdated Show resolved Hide resolved
docker/web_nginx.conf Outdated Show resolved Hide resolved
@mekarpeles mekarpeles marked this pull request as ready for review September 25, 2024 20:14
@mekarpeles mekarpeles merged commit b428d1f into internetarchive:master Sep 25, 2024
3 checks passed
d-dovale added a commit to d-dovale/openlibrary that referenced this pull request Sep 26, 2024
commit 80f511d
Merge: 3e50cc0 3ddd833
Author: jimchamp <28732543+jimchamp@users.noreply.github.com>
Date:   Thu Sep 26 11:04:06 2024 -0700

    Merge pull request internetarchive#9902 from cdrini/feature/toc-authors-etc

    Add authors, subtitle, and description to TOC on books page

commit 3ddd833
Author: Drini Cami <cdrini@gmail.com>
Date:   Mon Sep 23 21:37:01 2024 +0200

    Remove unused TableOfContents argument, highlighting

commit 8a03b7e
Author: Drini Cami <cdrini@gmail.com>
Date:   Mon Sep 23 21:32:51 2024 +0200

    Display author, subtitle, description of TOC on books page

commit 3e50cc0
Author: Benjamin Deitch <131627264+benbdeitch@users.noreply.github.com>
Date:   Thu Sep 26 08:56:46 2024 -0400

    Implementing Bulk Search v2 Designs (internetarchive#9851)

commit b428d1f
Author: Scott Barnes <scottreidbarnes@gmail.com>
Date:   Wed Sep 25 13:15:45 2024 -0700

    Nginx: add rate limiting (internetarchive#9904)

    * Nginx: add rate limiting

    See https://blog.nginx.org/blog/rate-limiting-nginx and
    https://nginx.org/en/docs/http/ngx_http_limit_req_module.html

    Adds comments in docker compose + web w/ instructions for re keeping nginx and docker replicas in sync

    ---------

    Co-authored-by: Mek <michael.karpeles@gmail.com>
DanielleInkster pushed a commit to DanielleInkster/openlibrary that referenced this pull request Oct 1, 2024
* Nginx: add rate limiting

See https://blog.nginx.org/blog/rate-limiting-nginx and
https://nginx.org/en/docs/http/ngx_http_limit_req_module.html

Adds comments in docker compose + web w/ instructions for re keeping nginx and docker replicas in sync

---------

Co-authored-by: Mek <michael.karpeles@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 1 Do this week, receiving emails, time sensitive, . [managed]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate auto-429ing high request IPs/DDOS
2 participants