Closed norton closed 9 years ago
New rename implementation for gdss-brick and gdss-client. Removed deprecated update and append API from gdss-client.
https://github.com/hibari/gdss-brick/commit/a55c96e67a2b47953b782ee8c8996e28d95023a9 https://github.com/hibari/gdss-client/commit/a8bdb5f010eaf1c1d3eac884ada1127ac536c0b6
New rename implementation for gdss-admin.
https://github.com/hibari/gdss-admin/commit/96ffa26edd73b726a46939a78071ce99a4929c75
Disable server implementation of rename.
https://github.com/hibari/gdss-brick/commit/5222e90851fa5cb59c76f9c78c81c06ec26619de
Prototype for this change request is on branch norton-server-rename.
I tried the prototype and found one issue. If I don't use a binary but string for the new key, the key value will be lost. I'll look after this issue.
Updated QuickCheck (qc) to detect the issue above. https://github.com/hibari/gdss-admin/commit/d101a50135d63289679590284f3813738f9b6946
To run qc, the repo manifest has to be updated. I'll do so after work.
I've been doing source code reivew and checked the write path for rename operation. I didn't find any obvious place to cause the issue on 7th comment.
<<"a">>
and "a"
are treated as different keys in Hibari.I think possible cause of the issue might be brick_ets:my_insert_ignore_logging/3
for insert_constant_value
(?VALUE_SWITCHAROO
) is not executed or silently fails if the new key is not binary. But how?
I'm going to enable the tracepoints to see the rename operation in action.
I fixed the issue described in 7th comment (https://github.com/hibari/gdss-brick/issues/2#issuecomment-13134534) and pushed the changes to norton-server-rename branch.
Commit: https://github.com/hibari/gdss-brick/commit/fe1091c
Cause:
In brick_server:make_rename/2
, the new key is not converted to binary while old key is done so. brick_server:make_op5
and make_op6
apply gmt_util:bin_ify/1
only to the old key, and brick_ets will silently fail to insert a key value if its key is not a binary.
Note: I was wrong in 10th comment (https://github.com/hibari/gdss-brick/issues/2#issuecomment-13809415). Keys are not stored "as is", but converted to binary just as I first thought. So my QuickCheck cases were actually correct.
Verification
Ran the QuickCheck cases in simple_eqc_tests
and verified that they passed 2,000 tests.
(hibariclient@127.0.0.1)2> simple_eqc_tests:run(2000).
prop_simple1:
...
Hibari: get_many() has been commented out from command()
.c......v....Yv....c.vvv.Y....vYY..v...c...cc.......Y.c.
...
OK, passed 2000 tests
51.05% {0,div10}
23.20% {1,div10}
12.45% {2,div10}
6.55% {3,div10}
3.45% {4,div10}
1.40% {5,div10}
0.75% {6,div10}
0.35% {7,div10}
0.30% {8,div10}
0.15% {11,div10}
0.15% {9,div10}
0.10% {10,div10}
0.05% {14,div10}
0.05% {13,div10}
prop_simple1_noproc_ok:
...
Hibari: get_many() has been commented out from command()
v.c...Y.v..v............vvcv.c..c.cY..v.c.Y.......Y..v..
...
OK, passed 2000 tests
50.60% {0,div10}
24.20% {1,div10}
12.05% {2,div10}
6.55% {3,div10}
2.75% {4,div10}
1.80% {5,div10}
1.05% {6,div10}
0.40% {7,div10}
0.35% {8,div10}
0.10% {11,div10}
0.10% {9,div10}
0.05% {10,div10}
[]
Merged into the dev branch: https://github.com/hibari/gdss-brick/issues/10#issuecomment-14013831
I decided the following things:
rename/6
on brick server with copy/6
, which is one-step simpler than rename and can be used as a building block of rename.rename/6
on the client API (brick_simple). It is just a convenient function to do do([make_txn(), make_copy(OldKey, NewKey, ...), make_delete(OldKey, ...)], ...)
.Target milestone is still v0.3.0.
(Edit)
Here is the link to GH issue of copy/6
.
As for the hibari issue 33 (bad sequence file 'enoent' error), ...
Right now, I have a prototype implementation with squidflash-primer working for a frozen hlog only on a head brick. I will continue work on it and implement squidflash-primer on downstream bricks (middle and official/ non-official tail bricks) as well.
I made a prototype of this (commit) but then realized this breaks modification replay in downstream bricks because it changes the order of modifications by delaying rename operation. I decided not to do squidflash-priming in downstream bricks. (I reverted the commit in the very next commit.)
I have decided to include this feature (rename/6
) into upcoming Hibari v0.1.11 release. I modified the codes on the dev branch so that brick_ets will act like hlog sequences are always frozen. I'll revisit this in next unstable major release v0.3.
gdss-brick >> GH2 - brick_server new client API - rename
rename/6 operation now always uses
rename_key_with_get_set_delete/6
instead ofmy_insert/6
with?KEY_SWITCHAPOO
. Will revisit this in Hibari v0.3 release.
Changing the milestone from v0.3 to v0.1.11.
Commit
gdss-brick >> GH2 - brick_server new client API - rename
rename/6 operation now always uses rename_key_with_get_set_delete/6 instead of my_insert/6 with ?KEY_SWITCHAPOO. Will revisit this in Hibari v0.3 release.
QuickCheck property prop_simple1
has found a bug in this implementation. If new key is overwriting an existing key-value with {exp_time_directive, keep}
and {attrib_directive, keep}
, rename_key_with_get_set_delete/6
will not only keep the exp_time and flags from old key (correct), but also from the existing key-value (wrong).
[{set,{var,1},
{call,simple_eqc_tests,simple_set,
[["/foo/bar/",["000",50]],
<<"v a">>,1426323338786789025,
[{exp_time_directive,keep}],
[{attrib2,"val2"}]]}},
{set,{var,2},
{call,simple_eqc_tests,simple_set,
[<<"/foo/bar/0001">>,<<"v a">>,1426323338789594039,
[{exp_time_directive,replace},{attrib_directive,keep}],
[]]}},
{set,{var,3},
{call,simple_eqc_tests,simple_rename,
[<<"/foo/bar/0001">>,
["/foo/bar/",["000",50]],
1426323338789723040,
[{exp_time_directive,keep},{attrib_directive,keep}],
[]]}},
{set,{var,4},
{call,simple_eqc_tests,simple_get,["/foo/bar/0002",[get_all_attribs]]}}]
S = {state,5,
[{<<"/foo/bar/0001">>,undefined},
{<<"/foo/bar/0002">>,1426323338789594039},
{<<"/foo/bar/0003">>,undefined},
{<<"/foo/bar/0004">>,undefined}],
[{<<"/foo/bar/0001">>,[]},
{<<"/foo/bar/0002">>,[<<"v a">>]},
{<<"/foo/bar/0003">>,[]},
{<<"/foo/bar/0004">>,[]}],
[{<<"/foo/bar/0001">>,undefined},
{<<"/foo/bar/0002">>,[]},
{<<"/foo/bar/0003">>,undefined},
{<<"/foo/bar/0004">>,undefined}],
[{tab1_ch1_b1,gdss_eunit@localhost}]}
R = {postcondition,false}
Hist = [{eqc_statem_history,
{state,1,
[{<<"/foo/bar/0001">>,undefined},
{<<"/foo/bar/0002">>,undefined},
{<<"/foo/bar/0003">>,undefined},
{<<"/foo/bar/0004">>,undefined}],
[{<<"/foo/bar/0001">>,[]},
{<<"/foo/bar/0002">>,[]},
{<<"/foo/bar/0003">>,[]},
{<<"/foo/bar/0004">>,[]}],
[{<<"/foo/bar/0001">>,undefined},
{<<"/foo/bar/0002">>,undefined},
{<<"/foo/bar/0003">>,undefined},
{<<"/foo/bar/0004">>,undefined}],
[{tab1_ch1_b1,gdss_eunit@localhost}]},
{call,simple_eqc_tests,simple_set,
[["/foo/bar/",["000",50]],
<<"v a">>,1426323338786789025,
[{exp_time_directive,keep}],
[{attrib2,"val2"}]]},
[],
{normal,{ok,1426323340565845}}},
{eqc_statem_history,
{state,2,
[{<<"/foo/bar/0001">>,undefined},
{<<"/foo/bar/0002">>,1426323338786789025},
{<<"/foo/bar/0003">>,undefined},
{<<"/foo/bar/0004">>,undefined}],
[{<<"/foo/bar/0001">>,[]},
{<<"/foo/bar/0002">>,[<<"v a">>]},
{<<"/foo/bar/0003">>,[]},
{<<"/foo/bar/0004">>,[]}],
[{<<"/foo/bar/0001">>,undefined},
{<<"/foo/bar/0002">>,[{attrib2,"val2"}]},
{<<"/foo/bar/0003">>,undefined},
{<<"/foo/bar/0004">>,undefined}],
[{tab1_ch1_b1,gdss_eunit@localhost}]},
{call,simple_eqc_tests,simple_set,
[<<"/foo/bar/0001">>,<<"v a">>,1426323338789594039,
[{exp_time_directive,replace},{attrib_directive,keep}],
[]]},
[],
{normal,{ok,1426323340572921}}},
{eqc_statem_history,
{state,3,
[{<<"/foo/bar/0001">>,1426323338789594039},
{<<"/foo/bar/0002">>,1426323338786789025},
{<<"/foo/bar/0003">>,undefined},
{<<"/foo/bar/0004">>,undefined}],
[{<<"/foo/bar/0001">>,[<<"v a">>]},
{<<"/foo/bar/0002">>,[<<"v a">>]},
{<<"/foo/bar/0003">>,[]},
{<<"/foo/bar/0004">>,[]}],
[{<<"/foo/bar/0001">>,[]},
{<<"/foo/bar/0002">>,[{attrib2,"val2"}]},
{<<"/foo/bar/0003">>,undefined},
{<<"/foo/bar/0004">>,undefined}],
[{tab1_ch1_b1,gdss_eunit@localhost}]},
{call,simple_eqc_tests,simple_rename,
[<<"/foo/bar/0001">>,
["/foo/bar/",["000",50]],
1426323338789723040,
[{exp_time_directive,keep},{attrib_directive,keep}],
[]]},
[],
{normal,{ok,1426323340580630}}},
{eqc_statem_history,
{state,4,
[{<<"/foo/bar/0001">>,undefined},
{<<"/foo/bar/0002">>,1426323338789594039},
{<<"/foo/bar/0003">>,undefined},
{<<"/foo/bar/0004">>,undefined}],
[{<<"/foo/bar/0001">>,[]},
{<<"/foo/bar/0002">>,[<<"v a">>]},
{<<"/foo/bar/0003">>,[]},
{<<"/foo/bar/0004">>,[]}],
[{<<"/foo/bar/0001">>,undefined},
{<<"/foo/bar/0002">>,[]},
{<<"/foo/bar/0003">>,undefined},
{<<"/foo/bar/0004">>,undefined}],
[{tab1_ch1_b1,gdss_eunit@localhost}]},
{call,simple_eqc_tests,simple_get,
["/foo/bar/0002",[get_all_attribs]]},
[],
{normal,
{ok,1426323340580630,<<"v a">>,1426323338786789025,
[{val_len,3},{attrib2,"val2"}]}}}]
QuickCheck property
prop_simple1
has found a bug in this implementation. If new key is overwriting an existing key-value with {exp_time_directive, keep} and {attrib_directive, keep}, rename_key_with_get_set_delete/6 will not only keep the exp_time and flags from old key (correct), but also from the existing key-value (wrong).
Fixed this by https://github.com/hibari/gdss-brick/commit/8c7fb13097f0cba0db49f5754a26849747ed59cb.
Now prop_simple1
passes 2,000 tests.
Erlang/OTP 17 [erts-6.3] [source] [64-bit] [smp:8:8] [async-threads:64] [hipe] [kernel-poll:true]
Eshell V6.3 (abort with ^G)
(hibariclient@127.0.0.1)1> eqc:quickcheck(eqc:numtests(2000, simple_eqc_tests:prop_simple1())).
Hibari: get_many() has been commented out from command()
Starting Quviq QuickCheck version 1.33.3
(compiled at {{2015,2,9},{14,18,24}})
...
vY.YY.Yv....c.....c....Y.......cv.Y.v..c.Y....v....Y.v......c.c..
...
OK, passed 2000 tests
49.45% {0,div10}
25.00% {1,div10}
12.20% {2,div10}
5.75% {3,div10}
3.05% {4,div10}
2.00% {5,div10}
0.85% {6,div10}
0.80% {7,div10}
0.40% {9,div10}
0.20% {10,div10}
0.20% {8,div10}
0.05% {16,div10}
0.05% {11,div10}
true
(hibariclient@127.0.0.1)2>
Fixed this by 8c7fb13. Now
prop_simple1
passes 2,000 tests.
Applied the same fix to branch gbrick-gh17-redesign-disk-storage
: https://github.com/hibari/gdss-brick/commit/e9957c5527d3b7e570e554947b0e74e69560276c
Rename a OldKey/Value pair to Key/Value pair in a brick, failing if OldKey does not already exist or if Key already exists. Similar to get_many, the rename API only works for keys of the same chain.