noahshaw11 commented on a change in pull request #3766:
URL: https://github.com/apache/couchdb/pull/3766#discussion_r775751772
##########
File path: src/smoosh/src/smoosh_channel.erl
##########
@@ -232,22 +232,24 @@ handle_info(unpause, State0) ->
State = maybe_open_queue(State0),
{noreply, maybe_start_compaction(State#state{paused = false})}.
-terminate(_Reason, #state{name = Name, waiting = Q}) ->
- file:delete(active_file_name(Name)),
- file:delete(starting_file_name(Name)),
- if
- Q =/= nil ->
- smoosh_priority_queue:close(Q);
- true ->
- nil
- end,
+terminate(_Reason, _State) ->
ok.
maybe_recover_state(#state{name = Name} = State) ->
Active = recover(active_file_name(Name)),
Starting = recover(starting_file_name(Name)),
- DatabaseDir = config:get("couchdb", "database_dir"),
- Active1 = get_matching_compact_files(DatabaseDir, Active),
+ Active1 = lists:foldl(
+ (fun({DbName, CPid}, Acc) ->
+ case couch_server:is_compacting(DbName) of
+ true ->
+ [{DbName, CPid} | Acc];
Review comment:
If we call `is_process_alive` for each `CPid`, I believe we will run
into the problem where we desync our `active` in the `state`. For example, if
we found a `CPid` that is not alive and call `enqueue` on the `DbName` for that
`CPid`, it will enqueue and record the `DbName` in `active`. However, at the
end of `maybe_recover_state` we overwrite the existing `active`:
```State#state{active = Active1, starting = Starting}```. Maybe it would be
safer to use the timestamp approach you recommended but in an "all or nothing"
approach where if the timestamps do not match then, we enqueue each DB that we
recovered from the state file.
--
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]
