Support listing file sources asynchronously #19256
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The method `list()` from `galaxy.files.sources.BaseFilesSource` lists the directories and files within a file source. An optional keyword argument `recursive` (`False` by default) lets it recursively retrieve directories and files within a specific directory. This operation is very cheap in terms of CPU and expensive in IO terms, be it network or filesystem IO. Depending on how the underlying system is built, it may support retrieving directories and files recursively or not. If it does not, then every time a directory is listed, it is necessary to make another request to list each subdirectory. This may end up involving hundreds of requests. Done sequentially, this can be extremely slow, especially if each one involves network access. This commit makes the `list()` method asynchronous, which enables Galaxy to wait for the underlying system to complete the requests concurrently, resulting in a massive speedup. The price to pay is the extra complexity of using the async primitives. Since this change implies that all functions in the chain up to the API endpoints and the test functions must also be made asynchronous, this commit also takes care of it.
4 tasks
4 tasks
kysrpex
added a commit
to kysrpex/galaxyproject-galaxy
that referenced
this pull request
Dec 16, 2024
…nc route thread FastAPI runs sync routes on a separate thread (from a threadpool), and async routes on the main thread's event loop. Originally, it was conceived to convert `_list` to an async method (see galaxyproject#19256). However, given that all other file source plugins are blocking, they could block the main thread for a significant amount of time (see fastapi/fastapi#3091). Thus, the implementation below creates a new event loop within the sync route's thread so that the eLabFTW plugin can still send concurrent requests without blocking the main thread.
|
This PR is no longer needed, see this comment. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The method
list()fromgalaxy.files.sources.BaseFilesSourcelists the directories and files within a file source. An optional keyword argumentrecursive(Falseby default) lets it recursively retrieve directories and files within a specific directory.This operation is very cheap in terms of CPU and expensive in IO terms, be it network or filesystem IO. Depending on how the underlying system is built, it may support retrieving directories and files recursively or not. If it does not, then every time a directory is listed, it is necessary to make another request to list each subdirectory. This may end up involving hundreds of requests. Done sequentially, this can be extremely slow, especially if each one involves network access.
This PR makes the
list()method asynchronous, which enables Galaxy to wait for the underlying system to complete the requests concurrently, resulting in a massive speedup. The price to pay is the extra complexity of using the async primitives.Since this change implies that all functions in the chain up to the API endpoints and the test functions must also be made asynchronous, this PR also takes care of it.
The changes from this PR are meant to address the friction that arises when integrating Galaxy with eLabFTW. Due to how the eLabFTW API is designed, listing the file source with
recursive=Truerequires sending one request for each experiment or resource that contains at least one attachment. Given a large enough eLabFTW instance, this easily translates to hundreds of requests, and the whole process takes an eternity when done serially. Making so many requests concurrently is still not ideal, but at least listing ~500 experiments or resources with attachments becomes bearable.This is the second PR of a series of PRs that integrate eLabFTW with Galaxy via a file source (together they address issue #18665):
How to test the changes?
(Select all options that apply)
License