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

[PR] Introduce a minimum batch size for auto purge [couchdb]

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

   ## Overview
   
   Use purge more efficiently by building up a queue and then issuing a batch at once. There is a minimum batch size (for efficiency) and a maximum (to prevent heavily conflicted documents forcing a very large purge request).
   
   ## Testing recommendations
   
   eunit
   
   ## Related Issues or Pull Requests
   
   https://github.com/apache/couchdb/pull/5655
   
   ## Checklist
   
   - [x] Code is written and works correctly
   - [x] Changes are covered by tests
   - [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
   - [ ] Documentation changes were made in the `src/docs` folder
   - [ ] Documentation changes were backported (separated PR) to affected branches
   


-- 
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] Introduce a minimum batch size for auto purge [couchdb]

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


##########
src/couch/src/couch_auto_purge_plugin.erl:
##########
@@ -73,47 +73,67 @@ doc_fdi(#{} = St, #full_doc_info{deleted = true} = FDI, Db) ->
     ?assert(
         FDI#full_doc_info.update_seq =< EndSeq, "FDI update_seq should not be greater than end seq"
     ),
-    {ok, purge(St, FDI, Db)};
+    {ok, enqueue(St, FDI, Db)};
 doc_fdi(#{} = St, #full_doc_info{}, _Db) ->
     {ok, St}.
 
-purge(#{} = St, #full_doc_info{} = FDI, Db) ->
+enqueue(#{} = St, FDI, Db) ->
     {Id, Revs} = fdi_to_idrevs(FDI),
-    MaxBatchSize = config:get_integer(atom_to_list(?MODULE), "max_batch_size", 500),
-    purge(St, Id, Revs, MaxBatchSize, Db).
+    enqueue(St, Id, Revs, Db).
 
-purge(#{} = St, Id, Revs, MaxBatchSize, Db) when length(Revs) =< MaxBatchSize ->
+enqueue(#{queue := Queue} = St0, Id, Revs, Db) ->
+    CurrentQueueSize = queue_size(Queue),
+    NewQueueSize = CurrentQueueSize + length(Revs),
+    MinBatchSize = min_batch_size(),
+    MaxBatchSize = max_batch_size(),
+
+    if
+        NewQueueSize > MaxBatchSize ->
+            {RevBatch, RevRest} = lists:split(NewQueueSize - MaxBatchSize, Revs),
+            St1 = flush_queue(St0#{queue => [{Id, RevBatch} | Queue]}, Db),
+            enqueue(St1, Id, RevRest, Db);
+        NewQueueSize >= MinBatchSize ->
+            flush_queue(St0#{queue => [{Id, Revs} | Queue]}, Db);
+        NewQueueSize < MinBatchSize ->
+            St0#{queue => [{Id, Revs} | Queue]}
+    end.
+
+flush_queue(#{queue := IdRevs} = St, Db) ->
     DbName = mem3:dbname(couch_db:name(Db)),
-    PurgeFun = fun() -> fabric:purge_docs(DbName, [{Id, Revs}], [?ADMIN_CTX]) end,
+    PurgeFun = fun() -> fabric:purge_docs(DbName, IdRevs, [?ADMIN_CTX]) end,

Review Comment:
   Nevermind the above comment, I was mistaken. Looking at the algorithm again, it I don't think we'll see duplicates. I added  a duplicates check in flush_queue and ran with a few small and large min/max limit and couldn't detect any that way either.



-- 
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] Introduce a minimum batch size for auto purge [couchdb]

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


##########
rel/overlay/etc/default.ini:
##########
@@ -1195,6 +1195,9 @@ url = {{nouveau_url}}
 ;[couch_quickjs_scanner_plugin.skip_{dbs,ddoc,docs}]
 
 [couch_auto_purge_plugin]
+; The fewest id/rev pairs the plugin will attempt to purge in
+; one request, excepting at the end of a database scan.
+;min_batch_size = 100

Review Comment:
   What do you think about increasing the default to about 200-250, so it's closer to about 50% of the maximum?



-- 
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] Introduce a minimum batch size for auto purge [couchdb]

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


##########
rel/overlay/etc/default.ini:
##########
@@ -1195,6 +1195,9 @@ url = {{nouveau_url}}
 ;[couch_quickjs_scanner_plugin.skip_{dbs,ddoc,docs}]
 
 [couch_auto_purge_plugin]
+; The fewest id/rev pairs the plugin will attempt to purge in
+; one request, excepting at the end of a database scan.
+;min_batch_size = 100

Review Comment:
   sure



-- 
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] Introduce a minimum batch size for auto purge [couchdb]

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


##########
src/couch/src/couch_auto_purge_plugin.erl:
##########
@@ -73,47 +73,67 @@ doc_fdi(#{} = St, #full_doc_info{deleted = true} = FDI, Db) ->
     ?assert(
         FDI#full_doc_info.update_seq =< EndSeq, "FDI update_seq should not be greater than end seq"
     ),
-    {ok, purge(St, FDI, Db)};
+    {ok, enqueue(St, FDI, Db)};
 doc_fdi(#{} = St, #full_doc_info{}, _Db) ->
     {ok, St}.
 
-purge(#{} = St, #full_doc_info{} = FDI, Db) ->
+enqueue(#{} = St, FDI, Db) ->
     {Id, Revs} = fdi_to_idrevs(FDI),
-    MaxBatchSize = config:get_integer(atom_to_list(?MODULE), "max_batch_size", 500),
-    purge(St, Id, Revs, MaxBatchSize, Db).
+    enqueue(St, Id, Revs, Db).
 
-purge(#{} = St, Id, Revs, MaxBatchSize, Db) when length(Revs) =< MaxBatchSize ->
+enqueue(#{queue := Queue} = St0, Id, Revs, Db) ->
+    CurrentQueueSize = queue_size(Queue),
+    NewQueueSize = CurrentQueueSize + length(Revs),
+    MinBatchSize = min_batch_size(),
+    MaxBatchSize = max_batch_size(),
+
+    if
+        NewQueueSize > MaxBatchSize ->
+            {RevBatch, RevRest} = lists:split(NewQueueSize - MaxBatchSize, Revs),
+            St1 = flush_queue(St0#{queue => [{Id, RevBatch} | Queue]}, Db),
+            enqueue(St1, Id, RevRest, Db);
+        NewQueueSize >= MinBatchSize ->
+            flush_queue(St0#{queue => [{Id, Revs} | Queue]}, Db);
+        NewQueueSize < MinBatchSize ->
+            St0#{queue => [{Id, Revs} | Queue]}
+    end.
+
+flush_queue(#{queue := IdRevs} = St, Db) ->
     DbName = mem3:dbname(couch_db:name(Db)),
-    PurgeFun = fun() -> fabric:purge_docs(DbName, [{Id, Revs}], [?ADMIN_CTX]) end,
+    PurgeFun = fun() -> fabric:purge_docs(DbName, IdRevs, [?ADMIN_CTX]) end,

Review Comment:
   I think there is a chance here that we'll get a duplicate doc_id in the same request. At the http api level the request is a  json object with a `{doc_id: [Rev]}` structure, so it implicitly doesn't have duplicates. Staring at the code in `couch_db_updater` it seems duplicate doc_ids should be handled, but it might be a tiny bit less efficient, since we'd do a partial purge, then another in subsequent {Id, Revs} request, as opposed to blowing away the FDI in one go. Maybe keeping the queue as a map of #{Id => Revs} could work? It might be less elegant though, I like how it looks clean and small now.



-- 
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] Introduce a minimum batch size for auto purge [couchdb]

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


-- 
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