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

feat(serverless,dashboard): limit maximum response time #733

Merged
merged 3 commits into from
Apr 7, 2023

Conversation

QuiiBz
Copy link
Member

@QuiiBz QuiiBz commented Apr 7, 2023

About

Closes #729

Based on the above PR description:

Now that we support parallel requests to the same deployment, the CPU time limit has been updated:

Previously, since each request was processed sequentially, it was easy to find how much time it takes to reply. We saved the start time, started a new thread, and terminate the isolate when start time + limit > now
Now, we can have multiple requests running in parallel, meaning instead of measuring the time it takes for each request, we measure the time elapsed between two (or more) event loop ticks. A thread is still spawned to terminate the isolate and close all ongoing requests if the event loop has been idle for too long.
The issue is that this limit anymore the maximum total execution time of the isolate. In the case of fetch() calls that take several seconds, and long JS execution, the isolate could technically keep running indefinitely assuming the event loop ticks regularly.

We should limit the total response time to 5 seconds for free plans, 30 seconds for pro plans, and custom for enterprise plans. We can use the start time of each request to calculate the elapsed time.

Add a timeout per request, based on the organization's plan:

  • 5s for the free plan
  • 30s for the pro plan

The timeout applies to both sync and stream responses.

@changeset-bot
Copy link

changeset-bot bot commented Apr 7, 2023

🦋 Changeset detected

Latest commit: f779933

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@lagon/docs Patch
@lagon/www Patch
@lagon/serverless Patch
@lagon/dashboard Patch

Not sure what this means? Click here to learn what changesets are.

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

@vercel
Copy link

vercel bot commented Apr 7, 2023

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

Name Status Preview Comments Updated (UTC)
dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 7, 2023 4:11pm
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
docs ⬜️ Ignored (Inspect) Visit Preview Apr 7, 2023 4:11pm
storybook ⬜️ Ignored (Inspect) Visit Preview Apr 7, 2023 4:11pm
www ⬜️ Ignored (Inspect) Visit Preview Apr 7, 2023 4:11pm

@QuiiBz QuiiBz merged commit a75de67 into main Apr 7, 2023
@QuiiBz QuiiBz deleted the feat/limit-maximum-response-time branch April 7, 2023 16:32
@QuiiBz QuiiBz mentioned this pull request Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limit maximum response time
1 participant