Skip to content

v2 IngestSendEvent rate limit #1134

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

Merged
merged 11 commits into from
May 31, 2024
Merged

v2 IngestSendEvent rate limit #1134

merged 11 commits into from
May 31, 2024

Conversation

matt-aitken
Copy link
Member

@matt-aitken matt-aitken commented May 28, 2024

Changes

  • Updated to Upstash's newest ratelimit package
  • Made it easy to create a rate limiter
  • Switch the API rate limiter to use the new RateLimiter class
  • Added a rate limiter to IngestSendEvent. It only queues the event if we're below the rate limit

What happens if the limit is exceeded

We return undefined from IngestSendEvent call() if the rate limit has been hit.

Practically this means:

  • DeliverScheduledEvent will throw an error causing a retry (max is 8 attempts)

For all other uses it will fail to do the runs, and will always log this message:

"IngestSendEvent: Rate limit exceeded"

Copy link

changeset-bot bot commented May 28, 2024

⚠️ No Changeset found

Latest commit: 72daae9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@matt-aitken matt-aitken requested a review from ericallam May 28, 2024 13:53
@ericallam ericallam force-pushed the ingest-events-rate-limit branch from 28a9542 to 72daae9 Compare May 31, 2024 08:50
@ericallam ericallam merged commit bc7bbd4 into main May 31, 2024
2 checks passed
@ericallam ericallam deleted the ingest-events-rate-limit branch May 31, 2024 09:00
jacobparis pushed a commit to jacobparis/trigger.dev that referenced this pull request Jun 1, 2024
* Easier to create a rate limiter, use it in the ApiRateLimiter. Upgraded the Upstash package

* Always prefix any rate limiter in Redis with “ratelimit:”

* By default log when the rate limit is hit

* Added rate limiting to IngestSendEvent

* Log out the EventRecord id

* Increase events.deliverScheduled attempts

* INGEST_EVENT_RATE_LIMIT_MAX is optional

* Removed old API rate limit code

* IngestSendEvent rate limiter is optional. Moved outside of the DB transaction

* Log a message out when the rate limiter is created

* Return undefined if the rate limit has been crossed
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.

2 participants