Use Key-value DB for DataStore and PinStore

[fetchadd]

This PR is for using key-value db like sled as DataStore and PinSotre, so that it is more efficient for pinning and easy to realize MFS.

The FsDatastore is inefficiency now, when checking a indirect pinned cid, all file in the FsDatastore will be visited and read, what more important is that it is hard to distinguish the data for pinning and the data for DataStore. The key-value db like sled will bring convenience and efficiency in the next work with its high efficiency and easy key/value operation. Leveldb may be the first choice of key-value dbs, but now there are no suitable rust leveldb crates; Sled is fast and simple, and it is usable on servers and phones from any C-compatible language.

[koivunej]

I think this is looking good. There's one comment regarding the unfortunate unsafe block but thanks a lot for doing this!

Using sled more in the future is something we'll want to heavily consider.

[koivunej]

Took me a while to decipher this but yeah ... init(&self) is an unfortunate wrinkle in the current design. Thanks for not changing this, this is much easier to review at least.. I cannot really see any workarounds for this at the moment even if this is quite unsafe.. Did you get any segfaults around this when testing? I wonder if there should at least be a memory barrier.

[fetchadd]

Yes, init(&self) brings a trouble. I got no segfaults when testing.

[koivunej]

It might be good enough for now to use mutex and clone the Db as in https://github.com/fetchadd/rust-ipfs/pull/1

[koivunej]

Did you try out the pinstore_interface_tests!, as in add this:

diff --git a/src/repo/kv.rs b/src/repo/kv.rs
index 19f0149..213b592 100644
--- a/src/repo/kv.rs
+++ b/src/repo/kv.rs
@@ -371,3 +371,6 @@ fn is_pinned(db: &KvDataStore, block: &Cid) -> Result<bool, Error> {
         Err(e) => Err(e),
     }
 }
+
+#[cfg(test)]
+crate::pinstore_interface_tests!(common_tests, crate::repo::kv::KvDataStore::new);

The macro will expand to a few test fns which aim to test the api behaviour of the store implementations, defined in common_tests.rs.

There are five successes and three failures:

test repo::kv::common_tests::cannot_recursively_unpin_unpinned ... FAILED
test repo::kv::common_tests::pin_direct_twice_is_good ... ok
test repo::kv::common_tests::indirect_can_be_pinned_directly ... ok
test repo::kv::common_tests::pin_recursive_pins_all_blocks ... ok
test repo::kv::common_tests::cannot_unpin_indirect ... FAILED
test repo::kv::common_tests::cannot_pin_recursively_pinned_directly ... FAILED
test repo::kv::common_tests::direct_and_indirect_when_parent_unpinned ... ok
test repo::kv::common_tests::can_pin_direct_as_recursive ... ok

I see there aren't too many comments in those common tests so please ask away if they are difficult to decipher. Could you take a look and see if the failures are quick to fix?

Overall I am thinking that the three failures might not be critical enough to block the PR as we will soon have largeish changes as we'll be able to update to tokio 1.0, so I'd rather get this PR in before those happen. Might be a good idea to put the sled behind a feature flag like with_sled as well at least until it passes the tests.

A final edit to combat any timezone induced lag: would you like to squash some commits away before this is merged, like the leveldb work which got replaced?

[fetchadd]

I am going to try to fix the failures and apply the changes to the newest master with no verbose commits..

[fetchadd]

Sorry for I don't known to how squash commits away from the branch in the PR, so I reopen it in #439 @koivunej

[koivunej]

Sorry for I don't known to how squash commits away from the branch in the PR, so I reopen it in #439

This is fine! For future reference, I usually do the git trickery with interactive rebase.