From a72d1cc9412b06bb12a3b49da4f7600ac10b25bd Mon Sep 17 00:00:00 2001 From: Martin Sumner Date: Tue, 5 Jun 2018 15:36:39 +0100 Subject: [PATCH 1/3] Validate current manifest was written Then delete a previous maniafest (from 5 manifests ago, if it is still there). --- src/leveled_pmanifest.erl | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/leveled_pmanifest.erl b/src/leveled_pmanifest.erl index 64699c8..fba14da 100644 --- a/src/leveled_pmanifest.erl +++ b/src/leveled_pmanifest.erl @@ -57,6 +57,7 @@ -define(TREE_TYPE, idxt). -define(TREE_WIDTH, 8). -define(PHANTOM_PID, r2d_fail). +-define(MANIFESTS_TO_RETAIN, 5). -record(manifest, {levels, % an array of lists or trees representing the manifest @@ -199,7 +200,21 @@ save_manifest(Manifest, RootPath) -> min_snapshot_sqn = 0, blooms = dict:new()}), CRC = erlang:crc32(ManBin), - ok = file:write_file(FP, <>). + ok = file:write_file(FP, <>), + {ok, <>} = file:read_file(FP), + GC_SQN = Manifest#manifest.manifest_sqn - ?MANIFESTS_TO_RETAIN, + % If a manifest is corrupted the previous one will be tried, so don't + % delete the previous one straight away. Retain until enough have been + % kept to make the probability of all being independently corrupted + % through separate events negligible + LFP = filepath(RootPath, GC_SQN, current_manifest), + ok = + case filelib:is_file(LFP) of + true -> + file:delete(LFP); + _ -> + ok + end. -spec replace_manifest_entry(manifest(), integer(), integer(), list()|manifest_entry(), From 93f16a6297b85af1e1b3c593f9126909df25092f Mon Sep 17 00:00:00 2001 From: Martin Sumner Date: Tue, 5 Jun 2018 16:48:56 +0100 Subject: [PATCH 2/3] Unit test - GC End up with right number of manifest files --- src/leveled_pmanifest.erl | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/leveled_pmanifest.erl b/src/leveled_pmanifest.erl index fba14da..51eb3bb 100644 --- a/src/leveled_pmanifest.erl +++ b/src/leveled_pmanifest.erl @@ -869,6 +869,9 @@ seconds_now() -> -ifdef(TEST). initial_setup() -> + initial_setup(single_change). + +initial_setup(Changes) -> E1 = #manifest_entry{start_key={i, "Bucket1", {"Idx1", "Fld1"}, "K8"}, end_key={i, "Bucket1", {"Idx1", "Fld9"}, "K93"}, filename="Z1", @@ -899,7 +902,10 @@ initial_setup() -> filename="Z6", owner="pid_z6", bloom=none}, - + initial_setup(Changes, E1, E2, E3, E4, E5, E6). + + +initial_setup(single_change, E1, E2, E3, E4, E5, E6) -> Man0 = new_manifest(), Man1 = insert_manifest_entry(Man0, 1, 1, E1), @@ -909,8 +915,20 @@ initial_setup() -> Man5 = insert_manifest_entry(Man4, 1, 2, E5), Man6 = insert_manifest_entry(Man5, 1, 2, E6), ?assertMatch(Man6, insert_manifest_entry(Man6, 1, 2, [])), + {Man0, Man1, Man2, Man3, Man4, Man5, Man6}; +initial_setup(multi_change, E1, E2, E3, E4, E5, E6) -> + Man0 = new_manifest(), + + Man1 = insert_manifest_entry(Man0, 1, 1, E1), + Man2 = insert_manifest_entry(Man1, 2, 1, E2), + Man3 = insert_manifest_entry(Man2, 3, 1, E3), + Man4 = insert_manifest_entry(Man3, 4, 2, E4), + Man5 = insert_manifest_entry(Man4, 5, 2, E5), + Man6 = insert_manifest_entry(Man5, 6, 2, E6), + ?assertMatch(Man6, insert_manifest_entry(Man6, 6, 2, [])), {Man0, Man1, Man2, Man3, Man4, Man5, Man6}. + changeup_setup(Man6) -> E1 = #manifest_entry{start_key={i, "Bucket1", {"Idx1", "Fld1"}, "K8"}, end_key={i, "Bucket1", {"Idx1", "Fld9"}, "K93"}, @@ -971,6 +989,18 @@ random_select_test() -> Level1 = array:get(1, LastManifest#manifest.levels), ?assertMatch(true, lists:member(L1File, Level1)). +manifest_gc_test() -> + RP = "../test_gc", + ok = filelib:ensure_dir(RP), + ManifestT = initial_setup(multi_change), + ManifestL = tuple_to_list(ManifestT), + lists:foreach(fun(M) -> save_manifest(M, RP) end, ManifestL), + {ok, FNs} = file:list_dir(filepath(RP, manifest)), + io:format("FNs ~w~n", [FNs]), + ?assertMatch(true, length(ManifestL) > ?MANIFESTS_TO_RETAIN), + ?assertMatch(?MANIFESTS_TO_RETAIN, length(FNs)). + + keylookup_manifest_test() -> {Man0, Man1, Man2, Man3, _Man4, _Man5, Man6} = initial_setup(), LK1_1 = {o, "Bucket1", "K711", null}, From fc998d2971825022e0d36b77a575d77a17385871 Mon Sep 17 00:00:00 2001 From: Martin Sumner Date: Tue, 5 Jun 2018 16:57:54 +0100 Subject: [PATCH 3/3] GC Inker Manifest --- src/leveled_imanifest.erl | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/leveled_imanifest.erl b/src/leveled_imanifest.erl index 76495f8..e619ab0 100644 --- a/src/leveled_imanifest.erl +++ b/src/leveled_imanifest.erl @@ -27,6 +27,7 @@ -define(MANIFEST_FILEX, "man"). -define(PENDING_FILEX, "pnd"). -define(SKIP_WIDTH, 16). +-define(MANIFESTS_TO_RETAIN, 5). -type manifest() :: list({integer(), list()}). %% The manifest is divided into blocks by sequence number, with each block @@ -169,9 +170,18 @@ writer(Manifest, ManSQN, RootPath) -> false -> leveled_log:log("I0016", [ManSQN]), ok = file:write_file(TmpFN, MBin), - ok = file:rename(TmpFN, NewFN), - ok - end. + ok = file:rename(TmpFN, NewFN) + end, + GC_SQN = ManSQN - ?MANIFESTS_TO_RETAIN, + GC_Man = filename:join(ManPath, + integer_to_list(GC_SQN) ++ "." ++ ?MANIFEST_FILEX), + ok = + case filelib:is_file(GC_Man) of + true -> + file:delete(GC_Man); + _ -> + ok + end. -spec printer(manifest()) -> ok. %% @doc