(couchdb) branch main updated: Make sure to first run cleanup, then kill the cleaner process

[va...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/couchdb.git


The following commit(s) were added to refs/heads/main by this push:
     new 1188d66c9 Make sure to first run cleanup, then kill the cleaner process
1188d66c9 is described below

commit 1188d66c9fe780371251b08a65ae58dc79870629
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Thu Aug 15 23:15:36 2024 -0400

    Make sure to first run cleanup, then kill the cleaner process
    
    Investigating streaming RPC worker timeouts noticed in our cleanup logic, there
    is a is tiny chance of leaking workers in case the coordinator process is
    killed right when it's running cleanup: after it stops the cleanup process and
    before it sends the `kill_all` messages.
    
    To fix it, first run cleanup, then kill the cleanup process.
---
 src/fabric/src/fabric_streams.erl | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/fabric/src/fabric_streams.erl b/src/fabric/src/fabric_streams.erl
index 85a4978c4..af2284f9f 100644
--- a/src/fabric/src/fabric_streams.erl
+++ b/src/fabric/src/fabric_streams.erl
@@ -95,8 +95,15 @@ start(Workers0, Keypos, StartFun, Replacements, RingOpts) ->
 
 cleanup(Workers) ->
     % Stop the auxiliary cleaner process as we got to the point where cleanup
-    % happesn in the regular fashion so we don't want to send 2x the number kill
-    % messages
+    % happens in the regular fashion and we don't want to send 2x the number
+    % of kill messages.
+    %
+    % First, we run the cleanup/1 function, then, we stop the cleaner;
+    % otherwise there is a tiny risk we get killed after we stop the process
+    % and before finish calling cleanup/1. This early, forced process kill may
+    % happen when running the recovery login in the ddoc cache.
+    %
+    Res = fabric_util:cleanup(Workers),
     case get(?WORKER_CLEANER) of
         CleanerPid when is_pid(CleanerPid) ->
             erase(?WORKER_CLEANER),
@@ -104,7 +111,7 @@ cleanup(Workers) ->
         _ ->
             ok
     end,
-    fabric_util:cleanup(Workers).
+    Res.
 
 handle_stream_start({rexi_DOWN, _, {_, NodeRef}, _}, _, St) ->
     #stream_acc{workers = Workers, ready = Ready, ring_opts = RingOpts} = St,