Feat/real rabin

[whyrusleeping]

This PR changes the chunking interface to allow it to return errors and also implements rabin chunking using a modified version of the implementation here: https://github.com/restic/chunker

[rht]

Is the block splitter to be used for parallel GetDag as well? (ah ic, GetBlocks already does every blocks concurrently)

[jbenet]

comments in

• whyrusleeping/chunker@0212082
• whyrusleeping/chunker@43301ee

[jbenet]

soooooo excited for this.

[jbenet]

@whyrusleeping we should drastically improve docker container space + transport usage after this lands :) ❤️ high priority.

[jbenet]

This LGTM. trying it out

[jbenet]

We should probably add a flag to the commands that use importer to try out rabin

[jbenet]

still to do:

• change the interface to take a min and max sizes. this avg thing is unwieldy.

[jbenet]

it's not wired up anywhere yet...

if this is already changing all of the interfaces, we should go for the io.Reader one. just do:

r := bytes.NewReader(buf)
return r, nil

for now, but that way we dont have to change the interfaces a ton of times.

[whyrusleeping]

the io.Reader interface made less and less sense the more i tried working it into the code. Every single place we would use it, we would just do an ioutil.ReadAll on the given reader, and assign those bytes to a dag.Node to be stored. and ReadAll overallocates memory so we dont want to do that.

[whyrusleeping]

I added an option to add that lets us specify a chunker:

ipfs add -s rabin              # default rabin parameters (256k average size)
ipfs add -s rabin-1000         # rabin with 256k avg block size
ipfs add -s rabin-100-500-1000 # rabin with 100 min blocksize, 500 average, 1000 max
ipfs add -s size-4000          # standard size chunking, 4000 byte blocks

[jbenet]

this is pretty cool, i like being able to specify options. should it be on the chunker or separate? i.e.:

ipfs add -s=rabin-1000-10000
ipfs add -s=rabin --rabin-min=1000 --rabin-max=10000

less typing, but also more clear.

i do like being able to express it in one string, but dont like the non-self-description. it may be worth expressing all chunckers in one string:

ipfs add -s=rabin-min:1000-max:10000

not sure. maybe the min: and max: parts could be optional?

btw, we can probably put the "string -> splitter with options" conversion in the splitter package, and use it in other tools. easy place to plug in other splitters over time.

also, "splitter" or "chunker" ? (last chance to rename). i like both.

[jbenet]

@whyrusleeping could you:

• consider making opts more expressive Feat/real rabin #1545 (you decide)
• consider splitter vs chunker (you decide)
• squash 408550e-3c0fbd7 as the tests dont pass before 3c0fbd7 ?

This LGTM and RFM! solid work! 👍

[jbenet]

@whyrusleeping also:

• rebase on latest master

[jbenet]

@whyrusleeping thouhgs on this?

• https://circleci.com/gh/ipfs/go-ipfs/904

[whyrusleeping]

@jbenet oh, thats just the random orientation of the data causing bad chunking boundaries, i can stop that from being a failure.

[jbenet]

@jbenet oh, thats just the random orientation of the data causing bad chunking boundaries, i can stop that from being a failure.

will randomness still make it fail sometimes?

[jbenet]

• https://travis-ci.org/ipfs/go-ipfs/jobs/74993590 <-- this is the infamous bitswap test hang again. we're so close to reliable tests-- maybe should push to fix this one.

[jbenet]

LGTM RFM