From 36264eb41683dd66215ddcff6eb38c01615b48eb Mon Sep 17 00:00:00 2001 From: Martin Sumner Date: Tue, 24 Oct 2017 13:19:30 +0100 Subject: [PATCH] Search range failure Discovered a bug with search ranges in leveled_tree - this was uncovered by an intermittently fialing 19.3 test. Test case added and bug fixed. It was due to a fialure to use end_key passed causing issues with particular manifests and full bucket ranges. --- src/leveled_log.erl | 2 ++ src/leveled_penciller.erl | 2 ++ src/leveled_pmanifest.erl | 43 +++++++++++++++++++++++++++++++++ src/leveled_tree.erl | 24 +++++++++++++++++- test/end_to_end/basic_SUITE.erl | 16 ++++++------ 5 files changed, 78 insertions(+), 9 deletions(-) diff --git a/src/leveled_log.erl b/src/leveled_log.erl index 59892dd..c3508b0 100644 --- a/src/leveled_log.erl +++ b/src/leveled_log.erl @@ -144,6 +144,8 @@ ++ "leaving SnapshotCount=~w and MinSQN=~w"}}, {"P0040", {info, "Archiving filename ~s as unused at startup"}}, + {"P0041", + {info, "Penciller manifest switched from SQN ~w to ~w"}}, {"PC001", {info, "Penciller's clerk ~w started with owner ~w"}}, diff --git a/src/leveled_penciller.erl b/src/leveled_penciller.erl index c726d08..8abe56b 100644 --- a/src/leveled_penciller.erl +++ b/src/leveled_penciller.erl @@ -673,6 +673,8 @@ handle_call(doom, _From, State) -> handle_cast({manifest_change, NewManifest}, State) -> NewManSQN = leveled_pmanifest:get_manifest_sqn(NewManifest), + OldManSQN = leveled_pmanifest:get_manifest_sqn(State#state.manifest), + leveled_log:log("P0041", [OldManSQN, NewManSQN]), ok = leveled_pclerk:clerk_promptdeletions(State#state.clerk, NewManSQN), UpdManifest = leveled_pmanifest:merge_snapshot(State#state.manifest, NewManifest), diff --git a/src/leveled_pmanifest.erl b/src/leveled_pmanifest.erl index 3970c7e..ba6e9b7 100644 --- a/src/leveled_pmanifest.erl +++ b/src/leveled_pmanifest.erl @@ -1128,6 +1128,49 @@ snapshot_timeout_test() -> Man10 = release_snapshot(Man9, ?PHANTOM_PID), ?assertMatch(0, length(Man10#manifest.snapshots)). +potential_issue_test() -> + Manifest = + {manifest,{array,9,0,[], + {[], + [{manifest_entry,{o_rkv,"Bucket","Key10",null}, + {o_rkv,"Bucket","Key12949",null}, + "<0.313.0>","./16_1_0.sst"}, + {manifest_entry,{o_rkv,"Bucket","Key129490",null}, + {o_rkv,"Bucket","Key158981",null}, + "<0.315.0>","./16_1_1.sst"}, + {manifest_entry,{o_rkv,"Bucket","Key158982",null}, + {o_rkv,"Bucket","Key188472",null}, + "<0.316.0>","./16_1_2.sst"}], + {idxt,1, + {{[{{o_rkv,"Bucket1","Key1",null}, + {manifest_entry,{o_rkv,"Bucket","Key9083",null}, + {o_rkv,"Bucket1","Key1",null}, + "<0.320.0>","./16_1_6.sst"}}]}, + {1,{{o_rkv,"Bucket1","Key1",null},1,nil,nil}}}}, + {idxt,0,{{},{0,nil}}}, + {idxt,0,{{},{0,nil}}}, + {idxt,0,{{},{0,nil}}}, + {idxt,0,{{},{0,nil}}}, + {idxt,0,{{},{0,nil}}}, + {idxt,0,{{},{0,nil}}}, + []}}, + 19,[],0, + {dict,0,16,16,8,80,48, + {[],[],[],[],[],[],[],[],[],[],[],[],[],[],[],[]}, + {{[],[],[],[],[],[],[],[],[],[],[],[],[],[],[],[]}}}, + 2}, + Range1 = range_lookup(Manifest, + 1, + {o_rkv, "Bucket", null, null}, + {o_rkv, "Bucket", null, null}), + Range2 = range_lookup(Manifest, + 2, + {o_rkv, "Bucket", null, null}, + {o_rkv, "Bucket", null, null}), + io:format("Range in Level 1 ~w~n", [Range1]), + io:format("Range in Level 2 ~w~n", [Range2]), + ?assertMatch(3, length(Range1)), + ?assertMatch(1, length(Range2)). -endif. diff --git a/src/leveled_tree.erl b/src/leveled_tree.erl index da171c9..8079f20 100644 --- a/src/leveled_tree.erl +++ b/src/leveled_tree.erl @@ -214,7 +214,7 @@ search_range(StartRange, EndRange, Tree, StartKeyFun) -> EndRangeFun = fun(ER, _FirstRHSKey, FirstRHSValue) -> StartRHSKey = StartKeyFun(FirstRHSValue), - ER >= StartRHSKey + not leveled_codec:endkey_passed(ER, StartRHSKey) end, case Tree of {tree, _L, T} -> @@ -405,8 +405,12 @@ idxtlookup_range_end(EndRange, {TLI, NK0, SL0}, Iter0, Output, EndRangeFun) -> [{FirstRHSKey, FirstRHSValue}|_Rest] -> case EndRangeFun(EndRange, FirstRHSKey, FirstRHSValue) of true -> + % The start key is not after the end of the range + % and so this should be included in the range Output ++ LHS ++ [{FirstRHSKey, FirstRHSValue}]; false -> + % the start key of the next key is after the end + % of the range and so should not be included Output ++ LHS end end; @@ -804,4 +808,22 @@ empty_test() -> T2 = empty(idxt), ?assertMatch(0, tsize(T2)). +search_range_idx_test() -> + Tree = + {idxt,1, + {{[{{o_rkv,"Bucket1","Key1",null}, + {manifest_entry,{o_rkv,"Bucket","Key9083",null}, + {o_rkv,"Bucket1","Key1",null}, + "<0.320.0>","./16_1_6.sst"}}]}, + {1,{{o_rkv,"Bucket1","Key1",null},1,nil,nil}}}}, + StartKeyFun = + fun(ME) -> + ME#manifest_entry.start_key + end, + R = search_range({o_rkv, "Bucket", null, null}, + {o_rkv, "Bucket", null, null}, + Tree, + StartKeyFun), + ?assertMatch(1, length(R)). + -endif. diff --git a/test/end_to_end/basic_SUITE.erl b/test/end_to_end/basic_SUITE.erl index 92ca46e..3c2e550 100644 --- a/test/end_to_end/basic_SUITE.erl +++ b/test/end_to_end/basic_SUITE.erl @@ -333,8 +333,8 @@ load_and_count(_Config) -> Bookie1, TestObject, G1), - {_S, Count} = testutil:check_bucket_stats(Bookie1, - "Bucket"), + {_S, Count} = + testutil:check_bucket_stats(Bookie1, "Bucket"), if Acc + 5000 == Count -> ok @@ -351,8 +351,8 @@ load_and_count(_Config) -> Bookie1, TestObject, G2), - {_S, Count} = testutil:check_bucket_stats(Bookie1, - "Bucket"), + {_S, Count} = + testutil:check_bucket_stats(Bookie1, "Bucket"), if Acc + 5000 == Count -> ok @@ -368,8 +368,8 @@ load_and_count(_Config) -> Bookie1, TestObject, G1), - {_S, Count} = testutil:check_bucket_stats(Bookie1, - "Bucket"), + {_S, Count} = + testutil:check_bucket_stats(Bookie1, "Bucket"), if Count == 200000 -> ok @@ -385,8 +385,8 @@ load_and_count(_Config) -> Bookie1, TestObject, G2), - {_S, Count} = testutil:check_bucket_stats(Bookie1, - "Bucket"), + {_S, Count} = + testutil:check_bucket_stats(Bookie1, "Bucket"), if Acc + 5000 == Count -> ok