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.

Sign up for GitHub

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

Jump to bottom

fix: client close blockstore on store directory and store blob #1198

Merged
merged 3 commits into from
Feb 3, 2022
Merged

fix: client close blockstore on store directory and store blob #1198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
85cd631
fix: client close blockstore on store directory and store blob
vasco-santos Jan 31, 2022
f775276
chore: update webstd fetch
vasco-santos Jan 31, 2022
cbc9eee
fix: make blockstore optional in encode functions
vasco-santos Jan 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/client/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"@ipld/car": "^3.1.20",
"@ipld/dag-cbor": "^6.0.13",
"@web-std/blob": "^3.0.1",
"@web-std/fetch": "^3.0.0",
"@web-std/fetch": "^3.0.3",
"@web-std/file": "^3.0.0",
"@web-std/form-data": "^3.0.0",
"carbites": "^1.0.6",
Expand Down
61 changes: 47 additions & 14 deletions packages/client/src/lib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const MAX_CHUNK_SIZE = 1024 * 1024 * 10 // chunk to ~10MB CARs
* @typedef {import('./lib/interface.js').Deal} Deal
* @typedef {import('./lib/interface.js').Pin} Pin
* @typedef {import('./lib/interface.js').CarReader} CarReader
* @typedef {import('ipfs-car/blockstore').Blockstore} BlockstoreI
*/

/**
Expand Down Expand Up @@ -99,9 +100,18 @@ class NFTStorage {
* @returns {Promise<CIDString>}
*/
static async storeBlob(service, blob) {
const { cid, car } = await NFTStorage.encodeBlob(blob)
await NFTStorage.storeCar(service, car)
return cid.toString()
const blockstore = new Blockstore()
let cidString

try {
const { cid, car } = await NFTStorage.encodeBlob(blob, { blockstore })
await NFTStorage.storeCar(service, car)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can as an alternative do this within storeCar, but we would need to provide blockstore here, which feels odd as we don't even use ipfs-car related things in the function. Also, wrapping everything to guarantee error handling looked the best option

Copy link

Choose a reason for hiding this comment

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

In my opinion, this can be redesigned as if a user want to store a big chunk, let's say 10GB, they need to have it available in tmpfs at once. I would recommend to have only the last pending blocks in the tmpfs and delete them after uploading.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are working on a new solution to pack files that will stream directly instead of using a temporary blockstore to keep the blocks. So, this will be an enhancement that we will ship soon to improve this behaviour #980

cidString = cid.toString()
} finally {
await blockstore.close()
}

return cidString
}

/**
Expand Down Expand Up @@ -177,9 +187,20 @@ class NFTStorage {
* @returns {Promise<CIDString>}
*/
static async storeDirectory(service, files) {
const { cid, car } = await NFTStorage.encodeDirectory(files)
await NFTStorage.storeCar(service, car)
return cid.toString()
const blockstore = new Blockstore()
let cidString

try {
const { cid, car } = await NFTStorage.encodeDirectory(files, {
blockstore,
})
await NFTStorage.storeCar(service, car)
cidString = cid.toString()
} finally {
await blockstore.close()
}

return cidString
}

/**
Expand Down Expand Up @@ -347,14 +368,18 @@ class NFTStorage {
* ```
*
* @param {Blob} blob
* @param {object} [options]
* @param {BlockstoreI} [options.blockstore]
* @returns {Promise<{ cid: CID, car: CarReader }>}
*/
static async encodeBlob(blob) {
static async encodeBlob(blob, { blockstore } = {}) {
if (blob.size === 0) {
throw new Error('Content size is 0, make sure to provide some content')
}

return packCar([{ path: 'blob', content: blob.stream() }], false)
return packCar([{ path: 'blob', content: blob.stream() }], {
blockstore,
wrapWithDirectory: false,
})
}

/**
Expand All @@ -378,9 +403,11 @@ class NFTStorage {
* ```
*
* @param {Iterable<File>} files
* @param {object} [options]
* @param {BlockstoreI} [options.blockstore]
* @returns {Promise<{ cid: CID, car: CarReader }>}
*/
static async encodeDirectory(files) {
static async encodeDirectory(files, { blockstore } = {}) {
const input = []
let size = 0
for (const file of files) {
Expand All @@ -394,7 +421,10 @@ class NFTStorage {
)
}

return packCar(input, true)
return packCar(input, {
blockstore,
wrapWithDirectory: true,
})
}

// Just a sugar so you don't have to pass around endpoint and token around.
Expand Down Expand Up @@ -604,10 +634,13 @@ For more context please see ERC-721 specification https://eips.ethereum.org/EIPS

/**
* @param {Array<{ path: string, content: import('./platform.js').ReadableStream }>} input
* @param {boolean} wrapWithDirectory
* @param {object} [options]
* @param {BlockstoreI} [options.blockstore]
* @param {boolean} [options.wrapWithDirectory]
*/
const packCar = async (input, wrapWithDirectory) => {
const blockstore = new Blockstore()
const packCar = async (input, { blockstore, wrapWithDirectory } = {}) => {
/* c8 ignore next 1 */
blockstore = blockstore || new Blockstore()
const { root: cid } = await pack({ input, blockstore, wrapWithDirectory })
const car = new BlockstoreCarReader(1, [cid], blockstore)
return { cid, car }
Expand Down
10 changes: 10 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4055,6 +4055,16 @@
"@web3-storage/multipart-parser" "^1.0.0"
data-uri-to-buffer "^3.0.1"

"@web-std/fetch@^3.0.3":
version "3.0.3"
resolved "https://registry.yarnpkg.com/@web-std/fetch/-/fetch-3.0.3.tgz#507e1371825298aae61172b0da439570437d3982"
integrity sha512-PtaKr6qvw2AmKChugzhQWuTa12dpbogHRBxwcleAZ35UhWucnfD4N+g3f7qYK2OeioSWTK3yMf6n/kOOfqxHaQ==
dependencies:
"@web-std/blob" "^3.0.3"
"@web-std/form-data" "^3.0.2"
"@web3-storage/multipart-parser" "^1.0.0"
data-uri-to-buffer "^3.0.1"

"@web-std/file-url@^1.0.1":
version "1.0.1"
resolved "https://registry.yarnpkg.com/@web-std/file-url/-/file-url-1.0.1.tgz#41209ec581ee7c97b19222b5daf47f2992f6fdd8"
Expand Down