bitswap: redo: don't re-provide blocks we've provided very recently

[kevina]

Pull request #3105 (bitswap: don't re-provide blocks we've provided very recently) broke adding files via the filestore (i.e., --no-copy, #2634). The filestore code requires that all writes go through to the blockstore. In addition the blockstore already checks if a block already exists. This change avoids calling Has() is the blockservice and instead modifies the blockstore interface to return the blocks actually added.
#3105 also changed AddBlocks (now AddObjects) to just return the blocks added. This pull request restores the original behavior. At this point I am not even sure if this return value is used anywhere.

[kevina]

@whyrusleeping if you don't like the idea of this pull request I am going to need to find another way around the problem. I will likely have to use my own BlockService when adding files via the filestore.

I also removed a comment you added later because I don't think it is relevant anymore, but I admit I do not 100% understand the comment is saying either.

[whyrusleeping]

Alright, finally getting to this one. We're gonna have to find a different way to do this. Changing the return signature like that shouldnt be necessary. You say you could use your own blockservice instead? what would that roughly look like?

[kevina]

I will basically need to duplicate BlockService and to provide a version of AddObject and AddObjects that don't do the Has check. They might also just not publish, but I'm not sure if that is the best idea.

[whyrusleeping]

Having a separate blockservice is okay. If you did that would you still need the whole multiblockstore thing?

[kevina]

Yes, What makes you think otherwise?

On Wed, 5 Oct 2016, Jeromy Johnson wrote:

Having a separate blockservice is okay. If you did that would you still need the whole multiblockstore thing?

You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
#3253 (comment)

[whyrusleeping]

I was thinking that all the logic youre doing with multiple blockstores could be abstracted into the blockservice. Your modified blockservice could have a blockstore, a filestore, and an exchange.

[kevina]

Changing the return signature like that shouldnt be necessary.

Note, that another reason I opted to changing the return type was to avoid calling Has() twice.

[kevina]

@whyrusleeping, please let me know if you are okay with, aab19fd. If you don't like the checkFirst flag in blockservice, I can have NewWriteThrough create a new class that reimplements AddObject/AddObjects at the expense of some code duplication and indirection.

I will make a new pull request once #3255 get merged. (I am sick of dealing with conflicts.)

[kevina]

closing in favor of #3294