Compare commits

...

2 commits

Author SHA1 Message Date
Shunichi Shinohara
393132a1d8 Minor type fix 2015-11-05 23:43:10 +09:00
Shunichi Shinohara
0e21581e47 Temporary dirty fix for filename reuse bug for AP-mode
Sequence numbers are only unique in the scope of single FLU, but not
unique (nor monotonic) in the scope of full chain. Additionally,
monotinicity in the scope of single FLU is good property for reusing
appendable files, simple randomness does not work.
Maybe, strict monotonicity is not necessary if most recent file names
for each prefix are tracked or trackable on demand.

This fix adds FLU name just before sequence number part.
Biggest downside is that it prevents file reuse to append by different
FLUs in CP-mode. Minor poor point is it does not work for FLU name like
`a-1'.
2015-11-05 23:42:53 +09:00
2 changed files with 40 additions and 27 deletions

View file

@ -67,12 +67,12 @@
]). ]).
-define(TIMEOUT, 10 * 1000). -define(TIMEOUT, 10 * 1000).
-include("machi_projection.hrl"). %% included for pv1_epoch_n type -include("machi_projection.hrl"). %% included for pv1_epoch type
-record(state, {fluname :: atom(), -record(state, {fluname :: atom(),
tid :: ets:tid(), tid :: ets:tid(),
datadir :: string(), datadir :: string(),
epoch :: pv1_epoch_n() epoch :: pv1_epoch()
}). }).
%% public API %% public API
@ -126,7 +126,7 @@ list_files_by_prefix(_FluName, Other) ->
init([FluName, DataDir]) -> init([FluName, DataDir]) ->
Tid = ets:new(make_filename_mgr_name(FluName), [named_table, {read_concurrency, true}]), Tid = ets:new(make_filename_mgr_name(FluName), [named_table, {read_concurrency, true}]),
{ok, #state{fluname = FluName, {ok, #state{fluname = FluName,
epoch = 0, epoch = {0, <<"NONE">>},
datadir = DataDir, datadir = DataDir,
tid = Tid}}. tid = Tid}}.
@ -138,19 +138,22 @@ handle_cast(Req, State) ->
%% the FLU has already validated that the caller's epoch id and the FLU's epoch id %% the FLU has already validated that the caller's epoch id and the FLU's epoch id
%% are the same. So we *assume* that remains the case here - that is to say, we %% are the same. So we *assume* that remains the case here - that is to say, we
%% are not wedged. %% are not wedged.
handle_call({find_filename, EpochId, Prefix}, _From, S = #state{ datadir = DataDir, handle_call({find_filename, EpochId, Prefix}, _From, S = #state{ fluname = FluName,
datadir = DataDir,
epoch = EpochId, epoch = EpochId,
tid = Tid}) -> tid = Tid}) ->
%% Our state and the caller's epoch ids are the same. Business as usual. %% Our state and the caller's epoch ids are the same. Business as usual.
File = handle_find_file(Tid, Prefix, DataDir), File = handle_find_file(Tid, Prefix, DataDir, FluName),
{reply, {file, File}, S}; {reply, {file, File}, S};
handle_call({find_filename, EpochId, Prefix}, _From, S = #state{ datadir = DataDir, tid = Tid }) -> handle_call({find_filename, EpochId, Prefix}, _From, S = #state{ fluname = FluName,
datadir = DataDir,
tid = Tid }) ->
%% If the epoch id in our state and the caller's epoch id were the same, it would've %% If the epoch id in our state and the caller's epoch id were the same, it would've
%% matched the above clause. Since we're here, we know that they are different. %% matched the above clause. Since we're here, we know that they are different.
%% If epoch ids between our state and the caller's are different, we must increment the %% If epoch ids between our state and the caller's are different, we must increment the
%% sequence number, generate a filename and then cache it. %% sequence number, generate a filename and then cache it.
File = increment_and_cache_filename(Tid, DataDir, Prefix), File = increment_and_cache_filename(Tid, DataDir, Prefix, FluName),
{reply, {file, File}, S#state{epoch = EpochId}}; {reply, {file, File}, S#state{epoch = EpochId}};
handle_call({increment_sequence, Prefix}, _From, S = #state{ datadir = DataDir }) -> handle_call({increment_sequence, Prefix}, _From, S = #state{ datadir = DataDir }) ->
@ -187,8 +190,8 @@ generate_uuid_v4_str() ->
io_lib:format("~8.16.0b-~4.16.0b-4~3.16.0b-~4.16.0b-~12.16.0b", io_lib:format("~8.16.0b-~4.16.0b-4~3.16.0b-~4.16.0b-~12.16.0b",
[A, B, C band 16#0fff, D band 16#3fff bor 16#8000, E]). [A, B, C band 16#0fff, D band 16#3fff bor 16#8000, E]).
find_file(DataDir, Prefix, N) -> find_file(DataDir, Prefix, FluName, N) ->
{_Filename, Path} = machi_util:make_data_filename(DataDir, Prefix, "*", N), {_Filename, Path} = machi_util:make_data_filename(DataDir, Prefix, "*", FluName, N),
filelib:wildcard(Path). filelib:wildcard(Path).
list_files(DataDir, Prefix) -> list_files(DataDir, Prefix) ->
@ -198,36 +201,39 @@ list_files(DataDir, Prefix) ->
make_filename_mgr_name(FluName) when is_atom(FluName) -> make_filename_mgr_name(FluName) when is_atom(FluName) ->
list_to_atom(atom_to_list(FluName) ++ "_filename_mgr"). list_to_atom(atom_to_list(FluName) ++ "_filename_mgr").
handle_find_file(Tid, Prefix, DataDir) -> handle_find_file(Tid, Prefix, DataDir, FluName) ->
N = machi_util:read_max_filenum(DataDir, Prefix), N = machi_util:read_max_filenum(DataDir, Prefix),
{File, Cleanup} = case find_file(DataDir, Prefix, N) of {File, Cleanup} =
[] -> case find_file(DataDir, Prefix, FluName, N) of
{find_or_make_filename(Tid, DataDir, Prefix, N), false}; [] ->
[H] -> {H, true}; {find_or_make_filename(Tid, DataDir, Prefix, FluName, N), false};
[Fn | _ ] = L -> [H] -> {H, true};
lager:debug( [_ | _] = L ->
"Searching for a matching file to prefix ~p and sequence number ~p gave multiples: ~p", lager:error(
[Prefix, N, L]), "Searching for a matching file to prefix ~p "
{Fn, true} "with flu name ~w and sequence number ~p gave multiples: ~p",
[Prefix, FluName, N, L]),
exit({bad_file_enties, {Prefix, FluName, N, L}})
end, end,
maybe_cleanup(Tid, {Prefix, N}, Cleanup), maybe_cleanup(Tid, {Prefix, N}, Cleanup),
filename:basename(File). filename:basename(File).
find_or_make_filename(Tid, DataDir, Prefix, N) -> find_or_make_filename(Tid, DataDir, Prefix, FluName, N) ->
case ets:lookup(Tid, {Prefix, N}) of case ets:lookup(Tid, {Prefix, N}) of
[] -> [] ->
F = generate_filename(DataDir, Prefix, N), F = generate_filename(DataDir, Prefix, FluName, N),
true = ets:insert_new(Tid, {{Prefix, N}, F}), true = ets:insert_new(Tid, {{Prefix, N}, F}),
F; F;
[{_Key, File}] -> [{_Key, File}] ->
File File
end. end.
generate_filename(DataDir, Prefix, N) -> generate_filename(DataDir, Prefix, FluName, N) ->
{F, _} = machi_util:make_data_filename( {F, _} = machi_util:make_data_filename(
DataDir, DataDir,
Prefix, Prefix,
generate_uuid_v4_str(), generate_uuid_v4_str(),
FluName,
N), N),
binary_to_list(F). binary_to_list(F).
@ -236,10 +242,10 @@ maybe_cleanup(_Tid, _Key, false) ->
maybe_cleanup(Tid, Key, true) -> maybe_cleanup(Tid, Key, true) ->
true = ets:delete(Tid, Key). true = ets:delete(Tid, Key).
increment_and_cache_filename(Tid, DataDir, Prefix) -> increment_and_cache_filename(Tid, DataDir, Prefix, FluName) ->
ok = machi_util:increment_max_filenum(DataDir, Prefix), ok = machi_util:increment_max_filenum(DataDir, Prefix),
N = machi_util:read_max_filenum(DataDir, Prefix), N = machi_util:read_max_filenum(DataDir, Prefix),
F = generate_filename(DataDir, Prefix, N), F = generate_filename(DataDir, Prefix, FluName, N),
true = ets:insert_new(Tid, {{Prefix, N}, F}), true = ets:insert_new(Tid, {{Prefix, N}, F}),
filename:basename(F). filename:basename(F).

View file

@ -32,7 +32,7 @@
make_regname/1, make_regname/1,
make_config_filename/2, make_config_filename/2,
make_checksum_filename/4, make_checksum_filename/2, make_checksum_filename/4, make_checksum_filename/2,
make_data_filename/4, make_data_filename/2, make_data_filename/5, make_data_filename/4, make_data_filename/2,
make_projection_filename/2, make_projection_filename/2,
is_valid_filename/1, is_valid_filename/1,
parse_filename/1, parse_filename/1,
@ -92,6 +92,13 @@ make_checksum_filename(DataDir, FileName) ->
%% @doc Calculate a file data file path, by common convention. %% @doc Calculate a file data file path, by common convention.
-spec make_data_filename(string(), string(), atom()|string()|binary(), atom(), integer()|string()) ->
{binary(), string()}.
make_data_filename(DataDir, Prefix, SequencerName, FluName, FileNum)
when is_atom(FluName) andalso is_integer(FileNum) ->
make_data_filename(DataDir, Prefix, SequencerName,
atom_to_list(FluName) ++ "-" ++ integer_to_list(FileNum)).
-spec make_data_filename(string(), string(), atom()|string()|binary(), integer()|string()) -> -spec make_data_filename(string(), string(), atom()|string()|binary(), integer()|string()) ->
{binary(), string()}. {binary(), string()}.
make_data_filename(DataDir, Prefix, SequencerName, FileNum) make_data_filename(DataDir, Prefix, SequencerName, FileNum)
@ -102,7 +109,7 @@ make_data_filename(DataDir, Prefix, SequencerName, FileNum)
make_data_filename(DataDir, Prefix, SequencerName, String) make_data_filename(DataDir, Prefix, SequencerName, String)
when is_list(String) -> when is_list(String) ->
File = erlang:iolist_to_binary(io_lib:format("~s^~s^~s", File = erlang:iolist_to_binary(io_lib:format("~s^~s^~s",
[Prefix, SequencerName, string])), [Prefix, SequencerName, String])),
make_data_filename2(DataDir, File). make_data_filename2(DataDir, File).
make_data_filename2(DataDir, File) -> make_data_filename2(DataDir, File) ->