Skip to content

Commit

Permalink
Fix del_table_copy loading loop
Browse files Browse the repository at this point in the history
When a local table copy was deleted but not yet loaded mnesia could
try to load it in a loop, which later when a table copy was added
again could result in a crash, since we don't have locks on the
receiver anymore. Tables can be simultaneously loaded and added.

Fixed by removing the table from the loader queue when it is
determined that the local copy is removed.

Also don't 'fatal' crash mnesia if table couldn't be loaded due
to table already existing.
  • Loading branch information
dgud committed Apr 24, 2024
1 parent d17819b commit 89f849f
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 28 deletions.
29 changes: 18 additions & 11 deletions lib/mnesia/src/mnesia_controller.erl
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,10 @@ handle_cast({adopt_orphans, Node, Tabs}, State) ->
end,
noreply(State2);

handle_cast({del_table_copy, Tab}, #state{late_loader_queue = LLQ0, loader_queue = LQ0} = State0) ->
noreply(State0#state{late_loader_queue = gb_trees:delete_any(Tab, LLQ0),
loader_queue = gb_trees:delete_any(Tab, LQ0)});

handle_cast(Msg, State) ->
error("~p got unexpected cast: ~tp~n", [?SERVER_NAME, Msg]),
noreply(State).
Expand Down Expand Up @@ -1224,19 +1228,19 @@ handle_info(Done = #loader_done{worker_pid=WPid, table_name=Tab}, State0) ->
false ->
ignore
end,
case ?catch_val({Tab, active_replicas}) of
[_|_] -> % still available elsewhere

case {?catch_val({Tab, storage_type}), val({Tab, active_replicas})} of
{unknown, _} -> %% Should not have a local copy anymore
State1#state{late_loader_queue=gb_trees:delete_any(Tab, LateQueue0)};
{_, [_|_]} -> % still available elsewhere
{value,{_,Worker}} = lists:keysearch(WPid,1,get_loaders(State0)),
add_loader(Tab,Worker,State1);
_ ->
{ram_copies, []} ->
DelState = State1#state{late_loader_queue=gb_trees:delete_any(Tab, LateQueue0)},
case ?catch_val({Tab, storage_type}) of
ram_copies ->
cast({disc_load, Tab, ram_only}),
DelState;
_ ->
DelState
end
cast({disc_load, Tab, ram_only}),
DelState;
{_, []} -> %% Table deleted or not loaded anywhere
State1#state{late_loader_queue=gb_trees:delete_any(Tab, LateQueue0)}
end
end,
State3 = opt_start_worker(State2),
Expand Down Expand Up @@ -1764,6 +1768,10 @@ del_active_replica(Tab, Node) ->
set(Var, mark_blocked_tab(Blocked, New)), % where_to_commit
mnesia_lib:del({Tab, active_replicas}, Node),
mnesia_lib:del({Tab, where_to_write}, Node),
case Node =:= node() of
true -> cast({del_table_copy, Tab});
false -> ok
end,
update_where_to_wlock(Tab).

change_table_access_mode(Cs) ->
Expand Down Expand Up @@ -2098,7 +2106,6 @@ opt_start_loader(State = #state{loader_queue = LoaderQ}) ->
true ->
opt_start_loader(State#state{loader_queue = Rest});
false ->
%% Start worker but keep him in the queue
Pid = load_and_reply(self(), Worker),
State#state{loader_pid=[{Pid,Worker}|get_loaders(State)],
loader_queue = Rest}
Expand Down
11 changes: 7 additions & 4 deletions lib/mnesia/src/mnesia_event.erl
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ handle_any_event(Msg, State) ->
{ok, State}.

handle_table_event({Oper, Record, TransId}, State) ->
report_info("~p performed by ~p on record:~n\t~tp~n",
report_info("~p performed by ~p on record:~n\t~0tp~n",
[Oper, TransId, Record]),
{ok, State}.

Expand Down Expand Up @@ -160,9 +160,12 @@ handle_system_event({mnesia_overload, Details}, State) ->
report_warning("Mnesia is overloaded: ~tw~n", [Details]),
{ok, State};

handle_system_event({mnesia_info, Format, Args}, State) ->
report_info(Format, Args),
{ok, State};
handle_system_event({mnesia_info, Format, Args} = Event, State) ->
case put(last, Event) of
Event -> ok;
_ -> report_info(Format, Args)
end,
{ok, State};

handle_system_event({mnesia_warning, Format, Args}, State) ->
report_warning(Format, Args),
Expand Down
36 changes: 23 additions & 13 deletions lib/mnesia/src/mnesia_loader.erl
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ val(Var) ->
disc_load_table(Tab, Reason, Cs) ->
Storage = mnesia_lib:cs_to_storage_type(node(), Cs),
Type = val({Tab, setorbag}),
dbg_out("Getting table ~tp (~p) from disc: ~tp~n",
dbg_out("Getting table ~0tp (~0p) from disc: ~0tp~n",
[Tab, Storage, Reason]),
?eval_debug_fun({?MODULE, do_get_disc_copy},
[{tab, Tab},
Expand All @@ -56,9 +56,9 @@ disc_load_table(Tab, Reason, Cs) ->
{type, Type}]),
do_get_disc_copy2(Tab, Reason, Storage, Type).

do_get_disc_copy2(Tab, _Reason, Storage, _Type) when Storage == unknown ->
verbose("Local table copy of ~tp has recently been deleted, ignored.~n",
[Tab]),
do_get_disc_copy2(Tab, Reason, Storage, _Type) when Storage == unknown ->
verbose("Local table copy of ~0tp ~0p has recently been deleted, ignored.~n",
[Tab, Reason]),
{not_loaded, storage_unknown};
do_get_disc_copy2(Tab, Reason, Storage, Type) when Storage == disc_copies ->
%% NOW we create the actual table
Expand Down Expand Up @@ -206,14 +206,14 @@ try_net_load_table(Tab, Reason, Ns, Cs) ->
end,
do_get_network_copy(Tab, Reason, Ns, Storage, Cs).

do_get_network_copy(Tab, _Reason, _Ns, unknown, _Cs) ->
verbose("Local table copy of ~tp has recently been deleted, ignored.~n", [Tab]),
do_get_network_copy(Tab, Reason, _Ns, unknown, _Cs) ->
verbose("Local table copy of ~0tp (~0p) has recently been deleted, ignored.~n", [Tab,Reason]),
{not_loaded, storage_unknown};
do_get_network_copy(Tab, Reason, Ns, Storage, Cs) ->
[Node | Tail] = Ns,
case lists:member(Node,val({current, db_nodes})) of
true ->
dbg_out("Getting table ~tp (~p) from node ~p: ~tp~n",
dbg_out("Getting table ~0tp (~0p) from node ~0p: ~0tp~n",
[Tab, Storage, Node, Reason]),
?eval_debug_fun({?MODULE, do_get_network_copy},
[{tab, Tab}, {reason, Reason},
Expand Down Expand Up @@ -289,6 +289,14 @@ init_receiver(Node, Tab,Storage,Cs,Reason) ->
{atomic, {error,Result}} when
element(1,Reason) == dumper ->
{error,Result};
{atomic, {error,{mktab, _} = Reason}} ->
case val({Tab,where_to_read}) == node() of
true -> %% Already loaded
ok;
false ->
fatal("Cannot create table ~tp: ~tp~n",
[[Tab, Storage], Reason])
end;
{atomic, {error,Result}} ->
fatal("Cannot create table ~tp: ~tp~n",
[[Tab, Storage], Result]);
Expand Down Expand Up @@ -415,26 +423,28 @@ create_table(Tab, TabSize, Storage, Cs) ->
{ok, _} ->
mnesia_lib:unlock_table(Tab),
{Storage, Tab};
Else ->
{error, Reason} ->
mnesia_lib:unlock_table(Tab),
Else
{error, {mktab, Reason}}
end;
(Storage == ram_copies) or (Storage == disc_copies) ->
EtsOpts = proplists:get_value(ets, StorageProps, []),
Args = [{keypos, 2}, public, named_table, Cs#cstruct.type | EtsOpts],
case mnesia_monitor:unsafe_mktab(Tab, Args) of
Tab ->
{Storage, Tab};
Else ->
Else
{error, Reason} ->
{error, {mktab, Reason}}
end;
element(1, Storage) == ext ->
{_, Alias, Mod} = Storage,
case mnesia_monitor:unsafe_create_external(Tab, Alias, Mod, Cs) of
ok ->
{Storage, Tab};
Else ->
Else
{error, Reason} ->
{error, {mktab, Reason}};
Reason ->
{error, {mktab, Reason}}
end
end.

Expand Down

0 comments on commit 89f849f

Please sign in to comment.