Skip to content

Configurable executor limit for download/upload tasks on FirebaseStorage #4445

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

GabrielAraujo
Copy link

@GabrielAraujo GabrielAraujo commented Dec 12, 2022

Right now this library only works with a limit of 2 threads for upload operations and 3 for download operations. With this limitation being fixed it can create problems for applications in which rely heavily on firebase storage for uploading/downloading a huge number of files at a time (Eg: 400+). This limitation in this scenario leads to a bad user experience when the users have to wait several minutes, even hours, and this is also a totally different experience that the iOS library provides too with more than 2/3 requests at a time.

The goal with this PR is to make it more configurable giving the developers the ability to manually adjust this limit on their applications if they want to!

Here are some threads related to this:
https://stackoverflow.com/questions/50917737/firebase-storage-limit-of-simultaneous-file-uploads-with-sdk
https://stackoverflow.com/questions/60163889/android-firebase-storage-sdk-maximum-parallel-uploads

@GabrielAraujo GabrielAraujo changed the title Configurable executor limit for download/upload tasks Configurable executor limit for download/upload tasks on FirebaseStorage Dec 12, 2022
@jamesdixon
Copy link

This would be great to have. Uploading a significant number of images through Firebase Storage on Android is quite slow compared to iOS.

Nice work @GabrielAraujo !

@GabrielAraujo
Copy link
Author

Hey! @vkryachko if you have time, would you mind taking a look at this one?

@vkryachko
Copy link
Member

I am afraid this change is not compatible with the current effort to migrate all SDKs to a common set of executors. So if this feature request gets approved, there will need to be a different way to limit concurrency without modifying the underlying executors, since common executors are "unconfigurable"

cc @maneesht

@GabrielAraujo
Copy link
Author

@vkryachko @maneesht Hey, suggestions on next steps for this?

@vkryachko
Copy link
Member

@GabrielAraujo Step 1 would be to migrate storage to executors, then we can use something like #4459(which will have to be productionized and merged first).

@jamesdixon
Copy link

@vkryachko has Storage been migrated to executors at this point?

@vkryachko
Copy link
Member

cc @rlazo

@rlazo
Copy link
Collaborator

rlazo commented Oct 8, 2023

@jamesdixon yes, it was migrated in #4830

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.

6 participants