More sled pinstore changes

[koivunej]

This PR adds:

• docs for the feature and the struct
• even longer TODOs, but with less FIXMEs
• pin key reuse in some cases from get_pinned_mode
• use TransactionalTree::flush instead of Db::flush_async, see sled pinstore tests deadlock rarely #443
• the use of spawn_blocking to do sled operations

Closes #443.

[koivunej]

It seems that cargo ndk is out with a new major version. I'll post another PR for fixing the pre-2.0 version. I am not so sure if we should continue building these, as we still are missing perhaps more important other builds.

[koivunej]

@fetchadd another follow-up. the changes are a bit a gnarly to read with some back and forth. If you are interested in reviewing and have trouble seeing whats going on, perhaps splitting the commit range at a2793f3 or the introduction of spawn_blocking is helpful.

[fetchadd]

I have read the codes, amazing changes has been made from the original 👍 . I have only one doubt now, that is the atomic of the list method. The is_pinned, insert_direct_pin... methods use transaction that do the operations like db.lock(); db.read_or_write(); db.unlock(), which make the sequence of operations be atomic and isolated. As the transaction of sled has no range to iter the keys, we have to use db.range, but it is not atomic and isolated, example like this can show this, so, will there be something bad?

By the way, a senior ruster is more appropriate for the review :).

[koivunej]

that is the atomic of the list method ... As the transaction of sled has no range to iter the keys

To be fair, I had missed this that there was no TransactionalTree::range. I figured that the atomic reads are not so bad, if we only ever update the tree through transactions. It might end up showing some pins which were added or removed during a listing, but at least from the http api point of view, it is more of an "introspection" use case than anything more serious.

For GC purposes, I am still thinking that an always-up-to-date set of collectable blocks is the way to go, given how I've understood go-ipfs aims to support this more of a stop the world process, but perhaps I am just naive as I haven't thought this through enough. The always up-to-date set of collectable blocks coupled with an LRU of trees would probably be the best solution, but I am guessing even either of those would work fine. Of course with an "LRU of trees" there's the issue of "which tree" in case a block is referenced from multiple but ... Perhaps a way can be found later on.

Also the current method has the issue of the unbounded channel which I had already noted. It's good that you brought this up, I added a bit more of comments in a4515f7.

By the way, a senior ruster is more appropriate for the review :)

I think you have a good POV as having now looked into at least leveldb and sled to comment on the matters, just to make sure the direction seems good. If there are any smaller bugs beign created that'd be a great opportunity to enhance the "common pinstore tests" as I've already noted in some FIXMEs. I'll start doing issues for the FIXMEs perhaps next following this PR.

[koivunej]

All right, lets get this in before #446 (not that the update was ready to go).

bors r+

[bors]

Build succeeded:

• ci