Refactor to support multi-store

[ncloudioj]

This is another try to fix #46 based on the discussion in #46.

This patch consists of following parts in 3 commits:

1. move Reader&Writer from Store to Rkv, also remove get function from Store
2. update all the examples
3. apply similar refactoring to integer store, note that this module appears to be a WIP, it needs some followup patches to support functionalities such as iterators

r? @mykmelez

[mykmelez]

Looks pretty good, only a few issues!

[mykmelez]

It isn't necessary to specify the concrete type for Writer/Reader keys in most cases, as the Rust compiler will infer it from calls to their methods. I only see three exceptions, all of which are in tests that retrieve a Reader and then never call a method on it that takes a key, which should be even rarer in the real world than it is in tests.

So I would leave off the turbofish key type annotations (::<&str>) except in those three cases, to improve readability.

(I also wonder how useful it is to constrain a whole Writer/Reader to a single concrete key type. We could enforce the constraint that keys are AsRef<[u8]> on each individual method that takes a key and never have to specify the key type when instantiating a Writer/Reader. But that's unrelated to this change, and we can consider it separately.)

[ncloudioj]

To be honest, I wasn't sure if Rust was smart enough to figure that out, should've given it a try, haha.

I was also trying to grok the point of having the K type (constrained as AsRef<[u8]>) in the code base, from LMDB's perspective, AsRef<[u8]> should be good enough. Perhaps there is some other considerations that I haven't yet grasped. We can take another look at this separately.

[mykmelez]

Nit: mutilpe -> multiple

[mykmelez]

Rkv::create_or_open_integer() still needs to return this type, and IntegerWriter/IntegerReader needs to require it. Otherwise, it'll be possible to retrieve a store with Rkv::create_or_open_integer() and then write to it with the non-integer Writer:

    #[test]
    fn test_integer_store_with_non_integer_keys() {
        let root = Builder::new().prefix("test_integer_store_with_non_integer_keys").tempdir().expect("tempdir");
        fs::create_dir_all(root.path()).expect("dir created");
        let k = Rkv::new(root.path()).expect("new succeeded");
        let s = k.open_or_create_integer("s").expect("open");

        let mut writer = k.write().expect("writer");

        writer.put(&s, "123", &Value::Str("hello!")).expect("write");
        assert_eq!(writer.get(&s, "123").expect("read"), Some(Value::Str("hello!")));
    }

Which compiles and runs but doesn't retrieve the written value:

> > cargo test
> …
> test integer::tests::test_integer_store_with_non_integer_keys ... FAILED
> …
> failures:
> 
> ---- integer::tests::test_integer_store_with_non_integer_keys stdout ----
> 	thread 'integer::tests::test_integer_store_with_non_integer_keys' panicked at 'assertion failed: `(left == right)`
>   left: `None`,

Conversely, it'll be possible to retrieve a store with Rkv::create_or_open() and then write to it with IntegerWriter, which compiles, runs, and retrieves the written value but probably isn't doing what the developer expects (i.e. applying LMDB's performance optimizations for integer-key stores).

The IntegerStore type enforces at compile time that all instances of IntegerWriter/IntegerReader are operating on a store that the developer retrieved via Rkv::create_or_open_integer(). I guess that still isn't entirely foolproof, as it's presumably still possible for the store in question to be a non-integer store (if it was previously created as such). But it's the equivalent of the current implementation.

(Ideally, I'd like Writer/Reader to be able to operate on both integer and non-integer stores, enforcing both of their key type constraints at compile time. But I'm not quite sure how to implement that. And in any case that's a separable problem. As you noted, IntegerStore is a WIP!)

[ncloudioj]

Nice catch, will take another look at this.

Ideally, I'd like Writer/Reader to be able to operate on both integer and non-integer stores, enforcing both of their key type constraints at compile time.

I was also thinking about that possibility in the process of this code refactoring. Having one reader/writer to take care of both types would be great, but my previous experience of dealing with LMDB integer store in Python and Go told me it'd be hard to implement. Now let's see if we can figure this out in Rust.

[ncloudioj]

Hmm, all the CI builds failed, and the failure doesn't make any sense to me.

Let me rebase against master, and try it again.

[ncloudioj]

OK, it passed the CI build after the rebase. I think we can revert the change in #51 now.

@mykmelez could you take another look at it?

[mykmelez]

OK, it passed the CI build after the rebase. I think we can revert the change in #51 now.

I updated the dependency to the 2018-07-18 nightly build in 8c2cc37. I've been thinking that we'd continue to pin rustfmt to specific versions to avoid getting busted by future breaking changes. Although it might end up being more work to update periodically to use new features. What do you think?

@mykmelez could you take another look at it?

Yup, looks great, thanks for the fixes!

[ncloudioj]

Thanks for reviewing this colossus patch!

Works for me in either way, so let's just leave it as it is for now. I believe rust-fmt will make its way to the official toolchain in the near future :)