rnewson commented on code in PR #5716:
URL: https://github.com/apache/couchdb/pull/5716#discussion_r2467049058
##########
src/couch/src/couch_auto_purge_plugin.erl:
##########
@@ -61,7 +61,7 @@ db_opened(#{} = St, Db) ->
true -> [{end_key, EndSeq}]
end,
?INFO("scanning for deleted documents in ~s up to ~p", [couch_db:name(Db),
EndSeq], meta(St)),
- {0, ChangeOpts, St#{count => 0, end_seq => EndSeq}}.
+ {0, ChangeOpts, St#{count => 0, end_seq => EndSeq, queue := []}}.
Review Comment:
I'd prefer an assertion that it was already empty, since we always flush the
queue in db_closing. e.g a match in the function head on `queue := []`.
##########
src/couch/src/couch_auto_purge_plugin.erl:
##########
@@ -89,19 +89,18 @@ enqueue(#{queue := Queue} = St0, Id, Revs, Db) ->
if
NewQueueSize > MaxBatchSize ->
{RevBatch, RevRest} = lists:split(MaxBatchSize - CurrentQueueSize,
Revs),
- St1 = flush_queue(St0#{queue => [{Id, RevBatch} | Queue]}, Db),
+ St1 = flush_queue(St0#{queue := [{Id, RevBatch} | Queue]}, Db),
enqueue(St1, Id, RevRest, Db);
NewQueueSize >= MinBatchSize ->
- flush_queue(St0#{queue => [{Id, Revs} | Queue]}, Db);
+ flush_queue(St0#{queue := [{Id, Revs} | Queue]}, Db);
Review Comment:
good idea on asserting these should already exist
##########
src/couch/src/couch_auto_purge_plugin.erl:
##########
@@ -89,19 +89,18 @@ enqueue(#{queue := Queue} = St0, Id, Revs, Db) ->
if
NewQueueSize > MaxBatchSize ->
{RevBatch, RevRest} = lists:split(MaxBatchSize - CurrentQueueSize,
Revs),
- St1 = flush_queue(St0#{queue => [{Id, RevBatch} | Queue]}, Db),
+ St1 = flush_queue(St0#{queue := [{Id, RevBatch} | Queue]}, Db),
enqueue(St1, Id, RevRest, Db);
NewQueueSize >= MinBatchSize ->
- flush_queue(St0#{queue => [{Id, Revs} | Queue]}, Db);
+ flush_queue(St0#{queue := [{Id, Revs} | Queue]}, Db);
NewQueueSize < MinBatchSize ->
- St0#{queue => [{Id, Revs} | Queue]}
+ St0#{queue := [{Id, Revs} | Queue]}
end.
flush_queue(#{queue := []} = St, _Db) ->
St;
-flush_queue(#{queue := IdRevs} = St, Db) ->
- DbName = mem3:dbname(couch_db:name(Db)),
- PurgeFun = fun() -> fabric:purge_docs(DbName, IdRevs, [?ADMIN_CTX]) end,
+flush_queue(#{queue := IdRevs, db_name := DbName, db_n := N} = St, Db) ->
+ PurgeFun = fun() -> fabric:purge_docs(DbName, IdRevs, [?ADMIN_CTX, {w,
N}]) end,
Review Comment:
agree on goal but would like to see this tidier
##########
src/couch/src/couch_auto_purge_plugin.erl:
##########
@@ -47,7 +47,7 @@ checkpoint(_St) ->
db(St, DbName) ->
case ttl(St, DbName) of
TTL when is_integer(TTL) ->
- {ok, St#{ttl => TTL}};
+ {ok, St#{ttl => TTL, db_name => DbName, db_n => mem3:n(DbName)}};
Review Comment:
I'd rather not clutter the state with information that the callbacks already
include, it's just more things to go wrong and we have to maintain.
--
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]
