Bz808 list keys opt#37
Conversation
Add an iterative process to try the three different API variations for folding over keys in various backends. IIRC there's a different bug/change-request to change these APIs to be more harmonious, so we'll tackle that bridge when we take it by the horns. Also added more extensive backend unit tests: for bitcask, cache, dets, ets, fs, and gb_trees backends. And added new-style folding API to the ETS backend, as an example (since it doesn't do anything differently than the fold implementation).
There was a problem hiding this comment.
What happened to this bitcask case for fold_keys? By my read this will now cause bitcask to fold over all data rather than keys now.
|
Bleh, my commit missed riak_kv_bitcask_backend.erl. It's pooooshed now. |
|
I get this with innostore master (1.0.3) as the backend ([email protected])1> systest:write(8). =ERROR REPORT==== 17-Feb-2011::14:26:39 === |
|
My listkeys test function listkeys(Size) -> listkeys(Start, End, Bucket) -> |
|
it looks like when innostore does innostore:fold_keys, they callback is expected to be fun(Key, Acc) instead of the fun(Key, _Val, Acc) being passed in. |
|
Yup, bad Scott, no cookie for you. ... I found a copy of your c("/Users/fritchie/b/src/systest/systest.erl"). |
|
The last wrinkle is that riak_kv_multi_backend does not pass through fold_bucket_keys. It works functionally but it means that list keys ends up doing full folds for innostore / bitcask which is suboptimal. This may be acceptable if we're going to do the backend refactor sooner rather than later... However I'm not sure we can count on it, even if we think we can. |
Add unit test for the multi backend, which was missing. Verify
via redbug that each of the actual backends is having the proper
fold function called: ets -> fold_bucket_keys, gb_trees -> fold.
19> redbug:start({riak_kv_ets_backend, fold_bucket_keys}).
ok
20> riak_kv_vnode:list_buckets_multi_backend_test().
ok
18:41:32 <{erlang,apply,2}> {riak_kv_ets_backend,fold_bucket_keys,
[<0.206.0>,'_',#Fun<riak_kv_vnode.2.64444309>,
[]]}
quitting: timeout
21> redbug:start({riak_kv_gb_trees_backend, fold_bucket_keys}).
ok
22> riak_kv_vnode:list_buckets_multi_backend_test().
ok
23> riak_kv_vnode:list_buckets_multi_backend_test().
ok
24> redbug:stop().
{stop,[]}
quitting: local_done
25> redbug:start({riak_kv_gb_trees_backend, fold}).
ok
26> riak_kv_vnode:list_buckets_multi_backend_test().
ok
18:42:02 <{erlang,apply,2}> {riak_kv_gb_trees_backend,fold,
[{state,<0.237.0>},
#Fun<riak_kv_vnode.2.64444309>,
[<<"f">>]]}
|
John, see 55b722d. I also added a list keys unit test for the multi |
|
Having different interfaces is so painful. It calls the correct fold_bucket_keys for bitcask, but not for innostore ([email protected])14> systest:listkeys(1, 1000, <<"i">>). With and ([email protected])1> riak_core_bucket:get_bucket(<<"b">>). |
Fix Jon's less-than-optimal bug as pointed out in pull req #37. Also, since the multi backend is actually bucket aware, we save work by only querying the one backend that we know is responsible for the bucket. Verifying, at console: {ok, C} = riak:local_client(). C:set_bucket(<<"i">>,[{backend, inno}]). C:set_bucket(<<"b">>,[{backend, bitcask}]). [C:put(riak_object:new(<<"i">>, <<X:32>>, <<X:32>>)) || X <- lists:seq(1,5)]. [C:put(riak_object:new(<<"b">>, <<X:32>>, <<X:32>>)) || X <- lists:seq(6,10)]. [C:put(riak_object:new(<<"x">>, <<X:32>>, <<X:32>>)) || X <- lists:seq(11,15)]. [{Bucket, C:list_keys(Bucket)} || Bucket <- [<<"i">>, <<"b">>, <<"x">>, <<"none">>]]. Config additions: --- rel/riak/etc/app.config 2011-02-17 16:50:36.000000000 -0600 +++ /tmp/app.config 2011-02-22 15:01:55.000000000 -0600 @@ -1,6 +1,11 @@ %% -*- tab-width: 4;erlang-indent-level: 4;indent-tabs-mode: nil -*- %% ex: ts=4 sw=4 et [ +{innostore, [ + {data_home_dir, "/tmp/innodb"}, %% Where data files go + {log_group_home_dir, "/tmp/innodb"}, %% Where log files go + {buffer_pool_size, 268435456} %% 256MB in-memory buffer in bytes +]}, %% Riak Core config {riak_core, [ %% Default location of ringstate @@ -30,7 +35,15 @@ {riak_kv, [ %% Storage_backend specifies the Erlang module defining the %storage %% mechanism that will be used on this node. - {storage_backend, riak_kv_bitcask_backend}, + %%{storage_backend, riak_kv_bitcask_backend}, +{storage_backend, riak_kv_multi_backend}, +{riak_kv_dets_backend_root, "data/dets"}, +{riak_kv_fs_backend_root, "data/fs"}, +{multi_backend_default, default}, +{multi_backend, [{default, riak_kv_dets_backend, + [{riak_kv_dets_backend_root, "data/dets"}]}, + {bitcask, riak_kv_bitcask_backend, []}, + {inno, riak_kv_innostore_backend, []}]}, %% pb_ip is the IP address that the Riak Protocol Buffers %interface %% will bind to. If this is undefined, the interface will not %run. ([email protected])1> {ok, C} = riak:local_client(). {ok,{riak_client,'[email protected]',<<1,158,32,8>>}} ([email protected])2> C:set_bucket(<<"i">>,[{backend, inno}]). ok ([email protected])3> C:set_bucket(<<"b">>,[{backend, bitcask}]). ok ([email protected])4> [C:put(riak_object:new(<<"i">>, <<X:32>>, <<X:32>>)) || X <- lists:seq(1,5)]. [ok,ok,ok,ok,ok] ([email protected])5> [C:put(riak_object:new(<<"b">>, <<X:32>>, <<X:32>>)) || X <- lists:seq(6,10)]. [ok,ok,ok,ok,ok] ([email protected])6> [C:put(riak_object:new(<<"x">>, <<X:32>>, <<X:32>>)) || X <- lists:seq(11,15)]. [ok,ok,ok,ok,ok] ([email protected])7> [{Bucket, C:list_keys(Bucket)} || Bucket <- [<<"i">>, <<"b">>, <<"x">>, <<"no">>]]. [{<<"i">>, {ok,[<<0,0,0,2>>, <<0,0,0,5>>, <<0,0,0,3>>, <<0,0,0,4>>, <<0,0,0,1>>]}}, {<<"b">>, {ok,[<<0,0,0,8>>, <<0,0,0,7>>, <<0,0,0,9>>, <<0,0,0,10>>, <<0,0,0,6>>]}}, {<<"x">>, {ok,[<<0,0,0,13>>, <<0,0,0,12>>, <<0,0,0,11>>, <<0,0,0,14>>, <<0,0,0,15>>]}}, {<<"no">>,{ok,[]}}]
|
+1 works for me - merge it! |
|
Fantastic. Any write up on this fix, I mean, I would like to see any performance improvement etc. |
Reviewer: anyone. Basic gist: using the first backend API function
that exists.