-
Notifications
You must be signed in to change notification settings - Fork 7
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
Don't load CAR file into memory during upload #193
Comments
How are you measuring this please? I believe the Node.js default is to allow up to ~4GB of memory to be allocated even if the currently in use memory is less. |
I am measuring this with This shows multiple threads, but if we just look at the top entry, it can be seen from the RES column that I don't think this is related to Node.js memory allocation settings. Please let me know if you are not able to reproduce this, as it seems like stable behaviour on my machine. Edit: To be clear, the file uploads fine. It works. But there is a bug here in the form of using large amounts of memory. |
@alanshaw any update on this? |
@makew0rld have you had any luck uploading even larger files? My colleague was running into time-out issues trying to upload a 32GB CAR file yesterday. After the 8th timeout he tried another CAR file that was 2.9GB, and that was successful on the first try. After some additional research, I did find this note that gives some details on how a CAR file larger than 4GB is split up into smaller chunks. I'm noticing in the command you run that you're not passing in the |
I haven't tested with larger files than 4 GB. |
Command:
w3 up --car my_file.car
.When running this command with a large file, for example a CAR file created from a 4 GiB file of random data, I noticed my memory usage would go up by ~4 GiB, indicating
w3
is loading the entire file into memory during the upload process.I'm not sure exactly where in the code this is coming from, but it's not ideal and could cause OOM crashes/issues in some scenarios. Please let me know if a fix for this is possible.
The text was updated successfully, but these errors were encountered: