feat: adjust shard size #908
Conversation
As discussed in #764 and calculated in https://observablehq.com/@gozala/w3up-shard-size, this PR adjusts the upload-client to shard on a CAR size of <= 133,169,152 bytes. It also fixes a bug where a CAR shard could be created with size greater than the shard size. supersedes #764
packages/upload-client/src/unixfs.js
Outdated
| await unixfsWriter.close() | ||
| try { | ||
| await fileBuilder.finalize(unixfsWriter) | ||
| await unixfsWriter.close() |
There was a problem hiding this comment.
Calling close on a writable stream that has errored causes this to throw an uncaught exception.
There was a problem hiding this comment.
☝️ This should probably be a code comment, also I don't think just logging an error is much better, browsers will log such errors and in node you can decide what to do. If we just log error things will appear to work but then break don't they ?
We should probably propagate error into returned readable somehow although I have to say I'm not sure how can we do that.
Any idea in what circumstances writable stream would error, it's not even exposed outside so if it errors perhaps it's problem in the UnixFS writer that can be fixed.
There was a problem hiding this comment.
I have removed and refactored the test so we don't encounter it.
What was causing it was the tests that were triggering the ShardingStream to error because a block is too big and it caused the CAR to exceed the shard size.
packages/upload-client/src/unixfs.js
Outdated
| await unixfsWriter.close() | ||
| try { | ||
| await fileBuilder.finalize(unixfsWriter) | ||
| await unixfsWriter.close() |
There was a problem hiding this comment.
☝️ This should probably be a code comment, also I don't think just logging an error is much better, browsers will log such errors and in node you can decide what to do. If we just log error things will appear to work but then break don't they ?
We should probably propagate error into returned readable somehow although I have to say I'm not sure how can we do that.
Any idea in what circumstances writable stream would error, it's not even exposed outside so if it errors perhaps it's problem in the UnixFS writer that can be fixed.
Co-authored-by: Irakli Gozalishvili <[email protected]>
Co-authored-by: Irakli Gozalishvili <[email protected]>
🤖 I have created a release *beep* *boop* --- ## [9.2.0](upload-client-v9.1.2...upload-client-v9.2.0) (2023-09-07) ### Features * adjust shard size ([#908](#908)) ([944182a](944182a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
As discussed in #764 and calculated in https://observablehq.com/@gozala/w3up-shard-size, this PR adjusts the
upload-clientto shard on a CAR size of <=133,169,152bytes.It also fixes a bug where a CAR shard could be created with size greater than the shard size.
supersedes #764