umgeher/leveled
0
Fork 0
Code Issues Pull requests Releases Activity Actions

Double-check safety of rolling memory

Browse source 
Make sure there is no change pending regardless of why maybe_roll_memory has been called.

Also, check that the manifest SQN has been incremented before accepting the change.

Conflict here would lead to data loss in the penciller, so extra safety is important.
This commit is contained in:
Martin Sumner 2019-02-26 10:33:20 +00:00
parent fd2e0e870c
commit 7dd07080c7
2 changed files with 48 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/leveled_log.erl
View file

@ -98,7 +98,7 @@
{"P0008",
{info, "Penciller closing for reason ~w"}},
{"P0010",
{info, "No level zero action on close of Penciller ~w"}},
{info, "No level zero action on close of Penciller discarded=~w"}},
{"P0011",
{info, "Shutdown complete for Penciller for reason ~w"}},
{"P0012",

85
src/leveled_penciller.erl
View file

@ -683,15 +683,12 @@ handle_call({push_mem, {LedgerTable, PushedIdx, MinSQN, MaxSQN}},
{reply, returned, State};
{false, true} ->
leveled_log:log("P0042", [State#state.levelzero_size]),
% The cache is full (there are 127 items already in it), so
% can't accept any more. However, we need to try and roll
% memory otherwise cache may be permanently full.
% The cache is full (the maximum line items have been reached), so
% can't accept any more. However, we need to try and roll memory
% otherwise cache may be permanently full.
gen_server:reply(From, returned),
{L0Pend, L0Constructor, none} =
maybe_roll_memory(State, false),
{noreply,
State#state{levelzero_pending=L0Pend,
levelzero_constructor=L0Constructor}};
{UpdState, none} = maybe_roll_memory(State, true, false),
{noreply, UpdState};
{false, false} ->
% leveled_log:log("P0018", [ok, false, false]),
PushedTree =
@ -909,18 +906,19 @@ handle_call(close, _From, State) ->
% on the clerk.
ok = leveled_pclerk:clerk_close(State#state.clerk),
leveled_log:log("P0008", [close]),
case State#state.levelzero_pending of
true ->
leveled_log:log("P0010", [State#state.levelzero_size]);
false ->
L0_Left = State#state.levelzero_size > 0,
case {State#state.levelzero_pending, L0_Left} of
{false, true} ->
{_L0Pend, L0Pid, _L0Bloom} = maybe_roll_memory(State, true),
{UpdState, _L0Bloom} = maybe_roll_memory(State, L0_Left, true),
L0Pid = UpdState#state.levelzero_constructor,
case is_pid(L0Pid) of
true ->
ok = leveled_sst:sst_close(L0Pid);
false ->
ok
end;
StatusTuple ->
leveled_log:log("P0010", [StatusTuple])
end
end,
shutdown_manifest(State#state.manifest, State#state.levelzero_constructor),
@ -958,14 +956,21 @@ handle_call(check_for_work, _From, State) ->
handle_call(persisted_sqn, _From, State) ->
{reply, State#state.persisted_sqn, State}.
handle_cast({manifest_change, NewManifest}, State) ->
NewManSQN = leveled_pmanifest:get_manifest_sqn(NewManifest),
handle_cast({manifest_change, Manifest}, State) ->
NewManSQN = leveled_pmanifest:get_manifest_sqn(Manifest),
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),
{noreply, State#state{manifest = UpdManifest, work_ongoing=false}};
% Only safe to update the manifest if the SQN increments
if NewManSQN > OldManSQN ->
ok =
leveled_pclerk:clerk_promptdeletions(State#state.clerk, NewManSQN),
% This is accepted as the new manifest, files may be deleted
UpdManifest =
leveled_pmanifest:merge_snapshot(State#state.manifest, Manifest),
% Need to preserve the penciller's view of snapshots stored in
% the manifest
{noreply, State#state{manifest=UpdManifest, work_ongoing=false}}
end;
handle_cast({release_snapshot, Snapshot}, State) ->
Manifest0 = leveled_pmanifest:release_snapshot(State#state.manifest,
Snapshot),
@ -1293,37 +1298,37 @@ update_levelzero(L0Size, {PushedTree, PushedIdx, MinSQN, MaxSQN},
false ->
true
end,
NoPendingManifestChange = not State#state.work_ongoing,
JitterCheck = RandomFactor or CacheMuchTooBig,
Due = CacheTooBig and JitterCheck,
case {Due, NoPendingManifestChange} of
{true, true} ->
{L0Pend, L0Constructor, none} =
maybe_roll_memory(UpdState, false),
LogSubs = [NewL0Size, true, true],
{UpdState0, _L0Bloom} = maybe_roll_memory(UpdState, Due, false),
LogSubs = [NewL0Size, Due, State#state.work_ongoing],
leveled_log:log_timer("P0031", LogSubs, SW),
UpdState#state{levelzero_pending=L0Pend,
levelzero_constructor=L0Constructor};
_ ->
LogSubs = [NewL0Size, Due, NoPendingManifestChange],
leveled_log:log_timer("P0031", LogSubs, SW),
UpdState
end
UpdState0
end.
-spec maybe_roll_memory(pcl_state(), boolean())
-> {boolean(), pid()|undefined, leveled_ebloom:bloom()|none}.
-spec maybe_roll_memory(pcl_state(), boolean(), boolean())
-> {pcl_state(), leveled_ebloom:bloom()|none}.
%% @doc
%% Check that no L0 file is present before rolling memory
maybe_roll_memory(State, SyncRoll) ->
%% Check that no L0 file is present before rolling memory. Returns a boolean
%% to indicate if memory has been rolled, the Pid of the L0 constructor and
%% The bloom of the L0 file (or none)
maybe_roll_memory(State, false, _SyncRoll) ->
{State, none};
maybe_roll_memory(State, true, SyncRoll) ->
BlockedByL0 = leveled_pmanifest:levelzero_present(State#state.manifest),
case BlockedByL0 of
PendingManifestChange = State#state.work_ongoing,
% It is critical that memory is not rolled if the manifest is due to be
% updated by a change by the clerk. When that manifest change is made it
% will override the addition of L0 and data will be lost.
case (BlockedByL0 or PendingManifestChange) of
true ->
{false, undefined, none};
{State, none};
false ->
{L0Constructor, Bloom} = roll_memory(State, SyncRoll),
{true, L0Constructor, Bloom}
{State#state{levelzero_pending=true,
levelzero_constructor=L0Constructor},
Bloom}
end.
-spec roll_memory(pcl_state(), boolean())