Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Commit

Permalink
fix: ensure correct progress is reported (#3384)
Browse files Browse the repository at this point in the history
Fixes a bug where due to parallel file imports, the reported size of
ingested bytes was getting mixed up.

We now tally imported bytes against the reported file name instead of
assuming a sequential import, taking care to not let the tally grow
until memory is exhausted.
  • Loading branch information
achingbrain authored Nov 12, 2020
1 parent 1311160 commit 633d870
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 20 deletions.
4 changes: 2 additions & 2 deletions docs/core-api/FILES.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ An optional object which may have the following keys:
| hashAlg | `String` | `'sha2-256'` | multihash hashing algorithm to use |
| onlyHash | `boolean` | `false` | If true, will not add blocks to the blockstore |
| pin | `boolean` | `true` | pin this object when adding |
| progress | function | `undefined` | a function that will be called with the byte length of chunks as a file is added to ipfs |
| progress | function | `undefined` | a function that will be called with the number of bytes added as a file is added to ipfs and the path of the file being added |
| rawLeaves | `boolean` | `false` | if true, DAG leaves will contain raw file data and not be wrapped in a protobuf |
| trickle | `boolean` | `false` | if true will use the [trickle DAG](https://godoc.org/github.com/ipsn/go-ipfs/gxlibs/github.com/ipfs/go-unixfs/importer/trickle) format for DAG generation |
| wrapWithDirectory | `boolean` | `false` | Adds a wrapping node around the content |
Expand Down Expand Up @@ -252,7 +252,7 @@ An optional object which may have the following keys:
| hashAlg | `String` | `'sha2-256'` | multihash hashing algorithm to use |
| onlyHash | `boolean` | `false` | If true, will not add blocks to the blockstore |
| pin | `boolean` | `true` | pin this object when adding |
| progress | function | `undefined` | a function that will be called with the number of bytes added as a file is added to ipfs and the name of the file being added |
| progress | function | `undefined` | a function that will be called with the number of bytes added as a file is added to ipfs and the path of the file being added |
| rawLeaves | `boolean` | `false` | if true, DAG leaves will contain raw file data and not be wrapped in a protobuf |
| shardSplitThreshold | `Number` | `1000` | Directories with more than this number of files will be created as HAMT-sharded directories |
| trickle | `boolean` | `false` | if true will use the [trickle DAG](https://godoc.org/github.com/ipsn/go-ipfs/gxlibs/github.com/ipfs/go-unixfs/importer/trickle) format for DAG generation |
Expand Down
21 changes: 11 additions & 10 deletions packages/interface-ipfs-core/src/add-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ module.exports = (common, options) => {
})

const emptyDir = (name) => ({ path: `test-folder/${name}` })
const progressSizes = {}

const dirs = [
content('pp.txt'),
Expand All @@ -152,20 +153,20 @@ module.exports = (common, options) => {
emptyDir('files/empty')
]

const total = dirs.reduce((i, entry) => {
return i + (entry.content ? entry.content.length : 0)
}, 0)
const total = dirs.reduce((acc, curr) => {
if (curr.content) {
acc[curr.path] = curr.content.length
}

return acc
}, {})

let progCalled = false
let accumProgress = 0
const handler = (p) => {
progCalled = true
accumProgress += p
const handler = (bytes, path) => {
progressSizes[path] = bytes
}

const root = await last(ipfs.addAll(dirs, { progress: handler }))
expect(progCalled).to.be.true()
expect(accumProgress).to.be.at.least(total)
expect(progressSizes).to.deep.equal(total)
expect(root.path).to.equal('test-folder')
expect(root.cid.toString()).to.equal(fixtures.directory.cid)
})
Expand Down
24 changes: 17 additions & 7 deletions packages/ipfs-core/src/components/add-all/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,19 @@ module.exports = ({ block, gcLock, preload, pin, options: constructorOptions })

delete opts.trickle

const totals = {}

if (opts.progress) {
let total = 0
const prog = opts.progress

opts.progress = (bytes, fileName) => {
total += bytes
prog(total, fileName)
opts.progress = (bytes, path) => {
if (!totals[path]) {
totals[path] = 0
}

totals[path] += bytes

prog(totals[path], path)
}
}

Expand All @@ -84,7 +90,12 @@ module.exports = ({ block, gcLock, preload, pin, options: constructorOptions })
const releaseLock = await gcLock.readLock()

try {
yield * iterator
for await (const added of iterator) {
// do not keep file totals around forever
delete totals[added.path]

yield added
}
} finally {
releaseLock()
}
Expand Down Expand Up @@ -179,8 +190,7 @@ function pinFile (pin, opts) {
* @property {boolean} [onlyHash=false] - If true, will not add blocks to the
* blockstore.
* @property {boolean} [pin=true] - Pin this object when adding.
* @property {(bytes:number, fileName:string) => void} [progress] - A function that will be
* called with the number of bytes added as a file is added to ipfs and the name of the file being added.
* @property {(bytes:number, path:string) => void} [progress] - a function that will be called with the number of bytes added as a file is added to ipfs and the path of the file being added
* @property {boolean} [rawLeaves=false] - If true, DAG leaves will contain raw
* file data and not be wrapped in a protobuf.
* @property {number} [shardSplitThreshold=1000] - Directories with more than this
Expand Down
2 changes: 1 addition & 1 deletion packages/ipfs-core/src/components/add.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ module.exports = ({ addAll }) => {
* @property {string} [hashAlg] - multihash hashing algorithm to use (default: `'sha2-256'`)
* @property {boolean} [onlyHash] - If true, will not add blocks to the blockstore (default: `false`)
* @property {boolean} [pin] - pin this object when adding (default: `true`)
* @property {Function} [progress] - a function that will be called with the byte length of chunks as a file is added to ipfs (default: `undefined`)
* @property {(bytes:number, path:string) => void} [progress] - a function that will be called with the number of bytes added as a file is added to ipfs and the path of the file being added
* @property {boolean} [rawLeaves] - if true, DAG leaves will contain raw file data and not be wrapped in a protobuf (default: `false`)
* @property {boolean} [trickle] - if true will use the [trickle DAG](https://godoc.org/github.com/ipsn/go-ipfs/gxlibs/github.com/ipfs/go-unixfs/importer/trickle) format for DAG generation (default: `false`)
* @property {boolean} [wrapWithDirectory] - Adds a wrapping node around the content (default: `false`)
Expand Down

0 comments on commit 633d870

Please sign in to comment.