nickva commented on code in PR #4813:
URL: https://github.com/apache/couchdb/pull/4813#discussion_r1418457489
##########
src/config/src/config.erl:
##########
@@ -315,6 +315,19 @@ handle_call({delete, Sec, Key, Persist, Reason}, _From,
Config) ->
_ ->
ok
end,
+ % If persistent, there will always be a delete marker just written
+ % in the last .ini file, so only reload in the non-persistent case.
+ Persist orelse
+ begin
+ IniMap = ini_map(Config#config.ini_files),
+ case maps:find({Sec, Key}, IniMap) of
+ % ?DELETE markers not inserted
+ {ok, Val} when is_list(Val) ->
+ true = ets:insert(?MODULE, {{Sec, Key}, Val});
+ _ ->
+ ok
+ end
Review Comment:
> It's definitely cleaner code to do it in both cases, but I thought you had
previously argued against reloading .ini files in the case of `Persist=false`?
I think it depends if we stay with delete markers. If we stay with delete
markers, then on `Persist=false` it doesn't matter if we reload, the effective
live value should still be an `undefined` (reload then still ets:delete is the
same as skipping reloading and just doing ets:delete).
If we get rid of delete markers, then reload/load/delete/set would all run
"load" to determine the live value. Persist is just a modifier whether we write
to file or not before running "load". There are additional bits in case of
reload to make sure to delete the ets values which now many have changed. There
is notification bit as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
