-
Notifications
You must be signed in to change notification settings - Fork 119
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: Bump MAX_BLOCK_SIZE to 2MiB limit imposed by bitswap #1269
Conversation
go-ipfs's chunker produces blocks containing 1MiB of data, however the protobuf wrapping each block results in ~0.001MiB of overhead. This means that when uploading CAR files containing blocks that go-ipfs produces, sometimes there are well-formed blocks that exceed the current limit. I am proposing bumping the max block size accepted by web3.storage to the limit imposed by ipfs bitswap, which seems like it's more in line with the reasoning behind imposing a block size limit at all. The 2MiB value selected is proposed by @Jorropo in the ipfs Discord, who claims that it's a part of the bitswap spec. This value is also mentioned in go-ipfs#3104.
@TheFrozenFire the go-ipfs chunker produces blocks of size 262,144 bytes by default. I don't understand what the issue is here? |
Not sure the exact context here, but for those looking for pointers around IPLD block sizes used in go-ipfs:
|
The background for why I'm asking for this change is that I'm currently running into errors with uploading content via the CAR API endpoint. I'm in the process of collecting all IPFS-type contentHashes that are referenced in Ethereum Name Service resolver records, merging them into an aggregate CID, and then replicating that to major pinning services. Since the high-level goal is to increase the availability/geodistribution of the content being served by Ethereum dApps, I need to maintain the original blocks. In doing this, I've come across a number of blocks which are slightly greater in size than the current limit of web3.storage. I can't speak to why the content creator chose to create 1MiB blocks, but these blocks are definitely "in the wild", so I think that it would be sensible to support them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given
I've come across a number of blocks which are slightly greater in size than the current limit of web3.storage
– @TheFrozenFire
and
with protobuf wrapping for those not using raw leaves the block sizes can then slightly exceed 1MiB 😭
– @aschmahmann
it sounds like we should increase the limit.
and with
The Bitswap spec is being clarified that it should tolerate blocks up to 2MiB...
then this seems like the right fix! Thanks @TheFrozenFire
Tests pass locally. |
Bitswap spec is being updated to clarify the 2MiB limit: ipfs/specs#269 go-ipfs chunker accepts a max size of 1MiB but protobuf wrapping can make it a bit bigger. Switching to 2MiB allows us to support blocks created using that max chunker size. refs web3-storage/web3.storage#1269
go-ipfs's chunker produces blocks containing 1MiB of data, however the protobuf wrapping each block results in ~0.001MiB of overhead. This means that when uploading CAR files containing blocks that go-ipfs produces, sometimes there are well-formed blocks that exceed the current limit. I am proposing bumping the max block size accepted by web3.storage to the limit imposed by ipfs bitswap, which seems like it's more in line with the reasoning behind imposing a block size limit.
The 2MiB value selected is proposed by @Jorropo in the ipfs Discord, who claims that it's a part of the bitswap spec. This value is also mentioned in ipfs/kubo#3104