amplify-js icon indicating copy to clipboard operation
amplify-js copied to clipboard

Storage.put slows application - require background disk based solution

Open sacrampton opened this issue 2 years ago • 16 comments

Before opening, please confirm:

JavaScript Framework

React Native

Amplify APIs

Storage

Amplify Categories

storage

Environment information

# Put output below this line
npx: installed 1 in 1.725s

  System:
    OS: Linux 4.14 Amazon Linux 2
    CPU: (2) x64 Intel(R) Xeon(R) CPU E5-2676 v3 @ 2.40GHz
    Memory: 6.41 GB / 7.79 GB
    Container: Yes
    Shell: 4.2.46 - /bin/bash
  Binaries:
    Node: 10.23.2 - ~/.nvm/versions/node/v10.23.2/bin/node
    npm: 6.14.10 - ~/.nvm/versions/node/v10.23.2/bin/npm
  npmGlobalPackages:
    @aws-amplify/cli: 6.3.1
    cdk: 1.87.1
    coffeescript: 2.5.1
    esformatter: 0.11.3
    js-beautify: 1.13.5
    npm: 6.14.10
    prettier: 2.2.1
    typescript: 3.7.5



Describe the bug

We would like to be able to use Storage.put primarily because we can use Cognito credentials to allow communication with S3. However, after years of trying to get this to work we have had to abandon it.

Initially we had photos our app would take sending in parallel - but given our customers could be offline for long periods of time we were finding storage.put would fill up memory and crash the app since it puts the photos into RAM memory prior to sending. So we changed the app to have it send files sequentially - slowed things down considerably, but at least didn't crash the app.

However, especially when coming back on line, process of loading the photos into memory and sending them would grind the app to a halt for unacceptably long periods of time. Anything more than 0 is unacceptable to me for the app to no be responsive and many of the GitHub issues around storage.put have times where the app is unresponsive. But we would have very long periods of time where it would be unresponsive - to the point clients would call it the "wheel of death" with the wait symbol would appear.

Eventually we abandoned the storage.put completely for https://www.npmjs.com/package/react-native-background-upload and that has removed all of the issues we had with storage.put grinding the app to a halt.

I have concerns about supportability with such a significant part of our app relying on something external to AWS. I am also not thrilled about using IAM credentials in our app rather than using Cognito. So longer term I would like storage.put to be fixed so it does not grind the application to a stand still.

Expected behavior

I would like storage.put to work without having to put the file into memory and to work as a background process so that files can be submitted in parallel without running the app out of memory and by putting it as a background process it does not consume resources of the device in sending files to S3.

Reproduction steps

See description above.

Code Snippet

// Put your code below this line.

Log output

// Put your logs below this line


aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

sacrampton avatar Mar 28 '22 02:03 sacrampton

Hi @chrisbonifacio @iartemiev - I have not had any feedback on this issue since creating this issue in over a month. This issue is essentially saying that in the strongest possible terms, it is not recommended to use amplify storage.put in any application unless the file is of negligible size (a few kb json file for example). So at the moment what I'm saying is that amplify does not have a storage solution that can be used for photos, audio, video because of the way it puts it into memory and grinds applications to a halt in that process. This means we have to use libraries that utilize IAM credentials rather than Cognito credentials which is problematic for a variety of reasons.

To me this is a critical issue where there is no usable solution for storage within amplify. If you are not able to do anything in the near term on this I think your documentation should be updated to recommend that people do not use amplify storage.put for anything other than trivial sized files because I know we wasted huge amounts of time on this issue.

sacrampton avatar May 03 '22 22:05 sacrampton

Hi @sacrampton,

Thanks for opening the issue and for all the detail you've provided. We have prioritized this and started investigating.

We'll update the thread with more details once we've reproduced the issue and looked into potential solutions to address it.

Ashish-Nanda avatar May 04 '22 16:05 Ashish-Nanda

@sacrampton

For helping us triage this issue, can you provide more details about:

  • Are you experience this problem on Web or Native Apps or both?
  • On which Device and Operating System version?
  • Which react-native version (package.json will be helpful as well)?
  • If is on Web, which browser and versions are you seen this problem?
  • What are you doing on your workaround (are you trying to use credentials provide from Auth category)?

Thanks!

elorzafe avatar May 04 '22 18:05 elorzafe

Hi @Ashish-Nanda @elorzafe - to answer your questions:

  • React Native App
  • Both iOS and Android on all versions (generally we only support latest versions - but it has been around for years on all versions)
  • I believe we are on react-native 0.65 (will confirm and update if that is not the case).
  • It is not on the web
  • Our work around is to remove storage.put entirely from our app and use the react-native-background-loader listed above. We have to provide the app the IAM credentials which is less than ideal.

The storage.put is particularly problematic when dealing with offline situations. Our app is used in a lot of offline / remote scenarios. This creates two major problems:

  • When offline for a while, storage.put keeps putting the photos in memory to send and eventually crashes the app by overflowing memory - so we had to change the logic of the app to send files serially rather than in parallel (ie. make sure one file has been sent, remove from memory, do the next file) so that we don't overflow memory when offline. There is no issue with this for react-native-background-loader - throw as many files as you want at that in online/offline mode.
  • When coming back on line you really notice the app hanging when it starts up and puts the files into memory to send it overtakes the app and you end up with a frozen screen until the files have uploaded.

Thanks for taking a look at this.

sacrampton avatar May 04 '22 19:05 sacrampton

Hi @sacrampton, thanks for responding with such detail so quickly. We are actively looking into this issue and we believe (and as you've shared) we are indeed running into the limitations of relying on what is essentially a full JS implementation of our Storage category for React Native. While this has served us well with happy path use cases that don't involve app backgrounding or large file sizes, your feedback is helping us drive a much deeper investigation into how we can (and should) leverage the native platforms unblock your very valid use cases. As we continue to look into the issue, I hope you can help me answer a few more questions that will help us to better understand what our ideal path forward should be.

  • Would it be at all possible to share with us the code snippet you were using to bring in files from the device file system into the JS runtime to be passed to storage.put?
  • As I am not super familiar with react-native-background-loader, could you help me understand
    • What you mean by "throw as many files as you want at that in online/offline mode"? Do you mean you are able to parallelize your uploads again?
    • How you get the file URL to be passed to the background uploader? (I could not find this in the documentation)

Even though we are actively looking into this issue, I don't think we can arrive at the best possible outcome without your contributions so please know that we really appreciate your help! Please rest assured that we will continue to make progress on resolving this issue but we have a lot of investigation still ahead of us including understanding the issue fully, evaluating solutions from the open source community and potentially working on our own native solutions. Thanks again!

Edit: Another quick question - when you said "It is not on the web" did you mean that your application is a mobile solution only or that the issue does not present on web?

cshfang avatar May 05 '22 18:05 cshfang

Hi @cshfang @Ashish-Nanda @elorzafe - I checked with our developers and their answers are listed below...

To get file uri/path we are using

  1. react-native-fs
  2. react-native-blob-util

There are two ways we used storage.put in our application.

a) Using fetch - but we are not using this currently because we were facing some ANR crash due to it.

Storage.configure({
	AWSS3: {
		bucket: awsMobile.aws_user_files_s3_bucket, //Your bucket name;
		region: awsMobile.aws_user_files_s3_bucket_region, //Specify the region your bucket was created in;
	},
});

const response = await fetch(uri);
const blob = await response.blob(); // format the data for images
const customPrefix = {
	public: '',
	protected: '',
	private: '',
};
const responseAfterUpload = await Storage.put(fileName, blob, {
	customPrefix: customPrefix,
	contentType: 'multipart/form-data',
});

b) By converting images into base64 - This is what we are using before using react-native-background-upload.

Storage.configure({
	AWSS3: {
		bucket: awsMobile.aws_user_files_s3_bucket, //Your bucket name;
		region: awsMobile.aws_user_files_s3_bucket_region, //Specify the region your bucket was created in;
	},
});

const base64 = await fs.readFile(path ? path : uri, 'base64');
const arrayBuffer = decode(base64);

const customPrefix = {
	public: '',
	protected: '',
	private: '',
};

sacrampton avatar May 06 '22 21:05 sacrampton

Thank you @sacrampton this is very helpful. I wonder if your engineers can recall whether you were running in the memory issue only during storage.put or if just reading the file in was having an impact on memory - Based on the code snippets you shared - it seems like reading file into a variable in JS would in and of itself be bringing the whole file into memory. This is a point of clarification that might help us in root causing the issue.

In the meantime, we are continuing on our end to reproduce the issue and identify possible solutions. Thank you for your continued support.

cshfang avatar May 09 '22 17:05 cshfang

Hi @cshfang, Yes I remember we had issue with fetch, when we try to fetch file from device using fetch and image uri we faced memory issue. We used this approach because official documentation suggest it. Screenshot 2022-05-10 at 6 37 36 PM .

rahulnainwal107 avatar May 10 '22 13:05 rahulnainwal107

@rahulnainwal107 thank you for confirming. We did suspect that the official docs will requires updates as well given our investigation thus far so it is helpful to have that verification.

cshfang avatar May 10 '22 13:05 cshfang

Hi @chintannp @cshfang @elorzafe @jamesaucode - just wanted to check if there has been any update in the last week that you can share.

sacrampton avatar May 19 '22 00:05 sacrampton

Hi @sacrampton , I am still investigating the issue and further exploring different issues involving fetch. I was not able to reproduce the issue on iOS while uploading a 900MB using fetch as mentioned in the docs here. Still have to test this on Android. Additionally, I still need to test this when uploading multiple larger files in parallel manner with limited internet connectivity. I'll get back to you with an update to the investigation and let you know about the findings.

chintannp avatar May 19 '22 21:05 chintannp

Hi @chintannp - just checking if there has been any progress on this in the last couple of weeks. Amplify Storage at present is not able to be used.

sacrampton avatar May 31 '22 00:05 sacrampton

Fetch works fine on iOS providing device has some good power behind it, super slow for older devices. For Android out of memory will throw.

I wasn't able to ever get upload working for Android using storage.put for videos, even if not so big in size. Tried a couple of times over the years.

The only way we could do so was by:

  1. Creating presigned s3 url
  2. Use react-native-background-upload to upload to the presigned url in the background - without multipart as haven't really been able to get that working (iOS upload speeds are slower unless in production version of app)

Note: this seems to upload slow in a development build, but once uploaded to TestFlight the upload speed is much faster.

Sample code:

Generate presigned url Lambda

   const params = {
          Bucket: process.env.STORAGE_XXXXX_BUCKETNAME,
          Key: input.key,
          ContentType: input.contentType,
          Expires: 900,
        };

        return s3.getSignedUrl('putObject', params).promise()

React native background upload

 const url = await generatePreSignedURL(key, contentType); // mutation that calls s3 presigned url lambda

    const options = {
      url,
      path,
      method: "PUT",
      type: "raw", // or raw
      maxRetries: 5, // set retry count (Android only). Default 2
      headers: {
        "Content-Type": contentType, // server requires a content-type header
        "Content-Length": `${fileInfo.size}`, // must be string - size is int
        bucket: awsmobile.aws_user_files_s3_bucket,
        region: awsmobile.aws_user_files_s3_bucket_region,
        key: `public/${key}`,
        // ACL: 'public-read'
      },
    };

    Upload.startUpload(options)
      .then((upId) => {
        setUploadId(upId);
        // event fired when upload progresses
        Upload.addListener("progress", upId, (data) => {
          let progress;
          const currProgress = data.progress / 100;
          console.log("currProgress: ", currProgress);
        });

        // event fired when upload error
        Upload.addListener("error", upId, (err) => {
          console.log("error with upload", err);
          if (err.error === "User cancelled upload") {
               // logic if user cancels upload
          } else {
            showErrorPage();
          }
        });

        // event when upload cancelled
        Upload.addListener("cancelled", upId, (data) => {
          console.log("upload cancelled", data);
        });

        // event when upload completes
        Upload.addListener("completed", upId, (data) => {
          const { responseCode, responseBody } = data;

          if (responseCode === 200) {
               // upload complete logic
          } else {
            console.log("upload response not okay", responseCode, data);
            // error
            showErrorPage();
          }
        });
      })
      .catch((err) => {
        console.log("Upload start error!", err);
        showErrorPage();
      });

dylan-westbury avatar Jun 02 '22 06:06 dylan-westbury

@sacrampton , Thanks for your patience. Below is the summary of my finding so far.

  1. fetch straight out does not work for android(for me) when uploading large files. Uploading smaller files(few MBs) works fine. However, when I try to upload a file 200mb+ it throws "Network request failed". There is a very detailed issue on this. Which I need to further explore. https://github.com/facebook/react-native/issues/28551#

  2. When using rn-fetch-blob package , uploading larger files still works but larger files(500 mb - 2 gb) will still hang the UI for around 500ms - 3 seconds. Additionaly, the UI main thread is also frozen when retrieving/verifying/updating credential during each consecutive data block upload.

  3. While I couldn't verify if the high memory usage is caused by fetch when reading larger files (because it throws network error), The main UI thread still hangs when Storage.put is uploading file chunks.

So, I believe there are 2 aspects that we need to work on.

  1. While rn-fetch-blob package works fine(with small UI thread hang ups) with reading the large files, we need to find out alternative solution which helps to solve the file reading issue(probably requires us to implement usage of native apis). Our goal would be to only keep chunks of the file in memory while it is uploading and sequentially remove them from memory once they are uploaded.

  2. Further explore and solve the issue why the UI thread hangs when retrieving/verifying/updating credential.

I will continue to work on the issue and engage with team to provide a solution for this issue.

chintannp avatar Jun 02 '22 16:06 chintannp

Hi @chintannp - glad to see you are making progress. Thanks for your efforts.

sacrampton avatar Jun 02 '22 21:06 sacrampton

Hi @sacrampton , I wanted to reach out and let you know that we are actively working on providing a solution for this.

chintannp avatar Aug 02 '22 21:08 chintannp

Hi @chintannp - just checking back to see if there is any further progress on this issue you can share?

sacrampton avatar Aug 21 '22 01:08 sacrampton

Hi @chintannp - given it has been almost 2 months since your most recent update I wanted to check back in to see what progress is being made. Thanks

sacrampton avatar Sep 26 '22 07:09 sacrampton

Hi @sacrampton - We are still working on this, and have a couple of approaches that we are experimenting with. The most sustainable path has been eluding us for a while, but we are feeling confident that we are marching towards a solution that we can share soon. We will link a PR to this issue when our solution is ready!

abdallahshaban557 avatar Sep 26 '22 23:09 abdallahshaban557

Hi, this is promising, will your solution include support for continuing long uploads when an app is backgrounded? At the moment they seem to be immediately suspended by the OS, but there are underlying APIs that would allow for this on iOS/Android. If not, would you consider providing and implementation of Storage.put that simply returns the pre-signed PUT URL (in a similar fashion that we can get a GET URL from the SDK? If we could access the pre-signed PUT URL for a given S3 key, we could leverage another background-friendly solution to do the actual uploading

aleksnied avatar Nov 09 '22 18:11 aleksnied

Any updates on this? We have users asking us for updates on when bugs will be fixed.

paulsizer avatar Dec 12 '22 10:12 paulsizer

Hi @paulsizer - we have a POC underway right now that we are testing out. We will provide more update when we have clarity on timelines!

abdallahshaban557 avatar Dec 13 '22 04:12 abdallahshaban557

Any updates? Currently blocking.

DanielWFrancis avatar Jan 26 '23 02:01 DanielWFrancis

+1 Any updates on this please?

vineethawal avatar Jan 27 '23 09:01 vineethawal

+1 too. Been waiting a while.

paulsizer avatar Jan 27 '23 09:01 paulsizer

+1; if anyone could work on this or give some guidance how to fix that, I'd really appreciate

VictorAtPL avatar Jan 27 '23 10:01 VictorAtPL

We apologize that this is taking a long time to fix - we still need more time to validate our approach. We will provide updates on this issue when we have a solid path forward.

abdallahshaban557 avatar Jan 27 '23 17:01 abdallahshaban557

+1, looking forward to any solution

interstates21 avatar Mar 21 '23 11:03 interstates21

+1

VictorAtPL avatar Mar 21 '23 11:03 VictorAtPL

+1

Has anyone successfully used aws-amplify with react-native-background-upload? It seems like others have moved away from aws-amplify altogether for apps where background uploading is a business req

AwakenedMind avatar May 28 '23 01:05 AwakenedMind