You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by "nickva (via GitHub)" <gi...@apache.org> on 2025/10/23 07:33:33 UTC

[PR] Improve index cleanup [couchdb]

nickva opened a new pull request, #5709:
URL: https://github.com/apache/couchdb/pull/5709

   Fix these issues:
   
     * Index cleanup triggered by smoosh only cleaned view indexes and purge checkpoints, make sure to also clean nouveau and search indexes and checkpoints.
   
     * `/_search_cleanup` endpoint only cleaned indexes on the coordinator node, despite being a clustered fabric endpoint. Fix it so it cleans indexes on other nodes as well as most users would expect.
   
     * Nouveau cleanup didn't clean purge checkpoints, so make it do so. Use the mrview purge checkpoint strategy for all indexes. This improves dreyfus logic to avoid traversing internal disk paths from clouseau.
   
     * Make `_view_cleanup` clean all the index types. This is a bit dirty, but it may be better than adding another cleanup `_index_cleanup` API. Left `_search_cleanup` and `_nouveau_cleanup` APIs as is for now.
   
   Some optimizations:
   
     * For each index clean request fetch ddocs once. Calculate signatures and then call the remote node cleanup logic with them. This avoids fetching design documents multiple times or sending all of them to the worker nodes. This is what Nouveau is doing so stick with that nice pattern.
   
     * Use erpc for remote calls. Our Erlang version is high enough (25+) to use the multple requests pattern from erpc. This is more compact than rexi. The absolute timeout pattern makes it simpler to have a global timeout for the whole request.
   
   Cleanups:
   
     * Make purge checkpoint fetching and cleanup more uniform. Use common utility logic in `couch_index_util` for all indexes.
   
     * Make index cleanup similar between all three indexes. Use the same erpc pattern for all of them.
   
     * Move index cleanup functions from fabric.erl to its own module -- fabric_index_cleanup.erl
   
     * Rename fabric function names more uniform and clearly indicate if it cleans indexes on all nodes or just the current node.
   
     * Add the db name to dreyfus `#index{}` record when it initializes so it matches Nouveau.
   


-- 
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: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Improve index cleanup [couchdb]

Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva merged PR #5709:
URL: https://github.com/apache/couchdb/pull/5709


-- 
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: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Improve index cleanup [couchdb]

Posted by "rnewson (via GitHub)" <gi...@apache.org>.
rnewson commented on code in PR #5709:
URL: https://github.com/apache/couchdb/pull/5709#discussion_r2454596193


##########
src/couch_mrview/src/couch_mrview_cleanup.erl:
##########
@@ -14,29 +14,37 @@
 
 -export([
     run/1,
-    cleanup_purges/3,
-    cleanup_indices/2
+    cleanup/2
 ]).
 
--include_lib("couch/include/couch_db.hrl").
-
 run(Db) ->
     Indices = couch_mrview_util:get_index_files(Db),
     Checkpoints = couch_mrview_util:get_purge_checkpoints(Db),
     {ok, Db1} = couch_db:reopen(Db),
     Sigs = couch_mrview_util:get_signatures(Db1),
-    ok = cleanup_purges(Db1, Sigs, Checkpoints),
+    ok = cleanup_purges(Db, Sigs, Checkpoints),

Review Comment:
   seem odd not to use Db1 after the reopen. explain?



##########
src/dreyfus/src/clouseau_rpc.erl:
##########
@@ -262,10 +262,11 @@ rename(DbName) ->
 %% and an analyzer represented in a Javascript function in a design document.
 %% `Sig` is used to check if an index description is changed,
 %% and the index needs to be reconstructed.
--spec cleanup(DbName :: string_as_binary(_), ActiveSigs :: [sig()]) ->
+-spec cleanup(DbName :: string_as_binary(_), SigMap :: #{sig() => true}) ->
     ok.
 
-cleanup(DbName, ActiveSigs) ->
+cleanup(DbName, #{} = SigMap) ->

Review Comment:
   a _search_cleanup during a cluster upgrade will crash then? I think that's fine as they are rare but might be worth noting somewhere.



-- 
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: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Improve index cleanup [couchdb]

Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on code in PR #5709:
URL: https://github.com/apache/couchdb/pull/5709#discussion_r2455444641


##########
src/dreyfus/src/clouseau_rpc.erl:
##########
@@ -262,10 +262,11 @@ rename(DbName) ->
 %% and an analyzer represented in a Javascript function in a design document.
 %% `Sig` is used to check if an index description is changed,
 %% and the index needs to be reconstructed.
--spec cleanup(DbName :: string_as_binary(_), ActiveSigs :: [sig()]) ->
+-spec cleanup(DbName :: string_as_binary(_), SigMap :: #{sig() => true}) ->
     ok.
 
-cleanup(DbName, ActiveSigs) ->
+cleanup(DbName, #{} = SigMap) ->

Review Comment:
   We can add an upgrade clause as well.  It's easy enough in this case



-- 
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: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Improve index cleanup [couchdb]

Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on code in PR #5709:
URL: https://github.com/apache/couchdb/pull/5709#discussion_r2455377535


##########
src/couch_mrview/src/couch_mrview_cleanup.erl:
##########
@@ -14,29 +14,37 @@
 
 -export([
     run/1,
-    cleanup_purges/3,
-    cleanup_indices/2
+    cleanup/2
 ]).
 
--include_lib("couch/include/couch_db.hrl").
-
 run(Db) ->
     Indices = couch_mrview_util:get_index_files(Db),
     Checkpoints = couch_mrview_util:get_purge_checkpoints(Db),
     {ok, Db1} = couch_db:reopen(Db),
     Sigs = couch_mrview_util:get_signatures(Db1),
-    ok = cleanup_purges(Db1, Sigs, Checkpoints),
+    ok = cleanup_purges(Db, Sigs, Checkpoints),

Review Comment:
   Good catch, should be Db1



-- 
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: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Improve index cleanup [couchdb]

Posted by "nickva (via GitHub)" <gi...@apache.org>.
nickva commented on code in PR #5709:
URL: https://github.com/apache/couchdb/pull/5709#discussion_r2455867529


##########
src/couch_mrview/src/couch_mrview_cleanup.erl:
##########
@@ -14,29 +14,37 @@
 
 -export([
     run/1,
-    cleanup_purges/3,
-    cleanup_indices/2
+    cleanup/2
 ]).
 
--include_lib("couch/include/couch_db.hrl").
-
 run(Db) ->
     Indices = couch_mrview_util:get_index_files(Db),
     Checkpoints = couch_mrview_util:get_purge_checkpoints(Db),
     {ok, Db1} = couch_db:reopen(Db),
     Sigs = couch_mrview_util:get_signatures(Db1),
-    ok = cleanup_purges(Db1, Sigs, Checkpoints),
+    ok = cleanup_purges(Db, Sigs, Checkpoints),

Review Comment:
   Updated



##########
src/dreyfus/src/clouseau_rpc.erl:
##########
@@ -262,10 +262,11 @@ rename(DbName) ->
 %% and an analyzer represented in a Javascript function in a design document.
 %% `Sig` is used to check if an index description is changed,
 %% and the index needs to be reconstructed.
--spec cleanup(DbName :: string_as_binary(_), ActiveSigs :: [sig()]) ->
+-spec cleanup(DbName :: string_as_binary(_), SigMap :: #{sig() => true}) ->
     ok.
 
-cleanup(DbName, ActiveSigs) ->
+cleanup(DbName, #{} = SigMap) ->

Review Comment:
   Added an upgrade clause



-- 
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: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org