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

fix: limit max upload size #224

Merged
merged 2 commits into from
Sep 13, 2023
Merged

fix: limit max upload size #224

merged 2 commits into from
Sep 13, 2023

Conversation

vasco-santos
Copy link
Contributor

@vasco-santos vasco-santos commented Sep 11, 2023

Adds limit based on previous decisions, considering that S3 Put command has hard-limit of 5GiB.

  • By limiting CAR size to 127*(1<<25), we guarantee max-4GiB-padded Filecoin pieces and have better utilization of Fil sector space.
  • By receiving one more byte, we would immediatly get to 8GiB padded piece.

Closes storacha/w3up#827

@seed-deploy
Copy link

seed-deploy bot commented Sep 11, 2023

View stack outputs

@seed-deploy seed-deploy bot temporarily deployed to pr224 September 12, 2023 09:28 Inactive
@vasco-santos
Copy link
Contributor Author

Also updated upload-api to enforce the limit right in the start

Copy link
Contributor

@Gozala Gozala left a comment

Choose a reason for hiding this comment

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

Looks good to me. I do think however that it might be a good idea to pull in the data-segment lib and use it's functions to derive this number. That way future changes will propagate without us having to find all the places we have calculated the sizes.

@vasco-santos
Copy link
Contributor Author

Looks good to me. I do think however that it might be a good idea to pull in the data-segment lib and use it's functions to derive this number. That way future changes will propagate without us having to find all the places we have calculated the sizes.

I would say this is implementation detail. Other users of data-segment can actually have other limits like 8Gb. I will merge but happy to iterate this further, even though I am not clear on what would mean using data-segment here. I guess starting from S3 limit of 5GB and find what is the bigger height that fit there. Afterwards, seeing what that size would map for a car size.

@vasco-santos vasco-santos merged commit 1466f0d into main Sep 13, 2023
1 check passed
@vasco-santos vasco-santos deleted the fix/limit-max-upload-size branch September 13, 2023 09: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.

Enforce max single put size to 127*(1<<25)
2 participants