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

set workers based on recommended number of cores #1170

Merged
merged 3 commits into from
Aug 27, 2024

Conversation

longshuicy
Copy link
Member

@longshuicy longshuicy commented Aug 20, 2024

Update:
To avoid the Namespace already exists, I turned off the recreate_views flag which prevents Beanie from attempting to recreate views if they already exist. This seems to work.


  1. I moved all the synchronous operations into a thread pool using the run_in_threadpool method.

    • This approach allows the synchronous tasks to be executed in a separate thread, preventing them from blocking the main asynchronous event loop.
    • You can read more about this approach here.
  2. More importantly, I realized we were not utilizing the maximum number of workers.

    • Uvicorn has built-in process management capabilities that can leverage multiple processes for better concurrency. You can read more about using workers here.
    • To set the number of workers, you can use the --workers {num_of_workers} option in Uvicorn.
    • Based on this recommendation, I configured 17 workers.

Expected behavior:

  • When downloading a large dataset, you can immediately navigate away from the page and still request other endpoints.
  • The backend will take some time to prepare the ZIP file.
  • Once the ZIP is ready and the streaming begins, the download will start automatically, regardless of which page you are on.
  • The browser will handle the download process

@tcnichol
Copy link
Contributor

I tested and made sure that the download would start if I immediately left the page. It did.

On my machine the browser starts being very slow after I click download. Would it perform better if I added more workers, or is it more a limitation on chrome or firefox?

@longshuicy longshuicy changed the base branch from main to release/v2.0-beta-3 August 21, 2024 19:39
Copy link
Contributor

@tcnichol tcnichol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and it worked. I was able to navigate away after clicking 'download' on a large dataset, and the dataset did download once the zip file was created.

@lmarini
Copy link
Member

lmarini commented Aug 27, 2024

Created #1189 to fix the work around.

@lmarini lmarini merged commit 8c9eb8a into release/v2.0-beta-3 Aug 27, 2024
6 checks passed
@lmarini lmarini deleted the large-file-stall-main-thread branch August 27, 2024 19:37
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.

Page is stuck if we try to download a dataset which contains big files
3 participants