janl commented on code in PR #5466:
URL: https://github.com/apache/couchdb/pull/5466#discussion_r2007304562
##########
rel/overlay/etc/default.ini:
##########
@@ -1055,6 +1056,20 @@ url = {{nouveau_url}}
; is to aggregate reports per database.
;ddoc_report = false
+[couch_scanner_plugin_conflict_finder]
+; Which types of conflicting docs to scan.
+;conflicts = true
+;deleted_conflicts = false
+
+; Run plugin on the first node (default) or on all nodes.
+;run_on_first_node = true
Review Comment:
this only works properly on single node and three node with n=3 setups,
right?
Otherwise the “first” node might not be holding all data. CC @nickva is this
a [limitation|deliberate design] of the scanner?
##########
rel/overlay/etc/default.ini:
##########
@@ -1055,6 +1056,20 @@ url = {{nouveau_url}}
; is to aggregate reports per database.
;ddoc_report = false
+[couch_scanner_plugin_conflict_finder]
+; Which types of conflicting docs to scan.
+;conflicts = true
+;deleted_conflicts = false
+
+; Run plugin on the first node (default) or on all nodes.
+;run_on_first_node = true
+
+; Emit reports for each conflicted doc or aggregate them per database.
+; If doc_report is set to true, the report will indicate doc name and revs
info.
+; Otherwise, only conflicted doc numbers are accumulated.
+; The default is to aggregate reports per database without revs info.
+;doc_report = false
Review Comment:
This needs discussion. I can see how getting the aggregates first is useful
to keep log spam at bay, but as soon as there are entries, you wanna swap this
and then you have to wait for another scan. It feels like just showing the
conflicts directly is the better default.
##########
rel/overlay/etc/default.ini:
##########
@@ -1055,6 +1056,20 @@ url = {{nouveau_url}}
; is to aggregate reports per database.
;ddoc_report = false
+[couch_scanner_plugin_conflict_finder]
+; Which types of conflicting docs to scan.
+;conflicts = true
+;deleted_conflicts = false
Review Comment:
```suggestion
;deleted_conflicts = true
```
let’s include these by default
##########
src/couch_scanner/src/couch_scanner_plugin_conflict_finder.erl:
##########
@@ -0,0 +1,196 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_scanner_plugin_conflict_finder).
+-behaviour(couch_scanner_plugin).
+
+-export([
+ start/2,
+ resume/2,
+ complete/1,
+ checkpoint/1,
+ db/2,
+ doc_id/3
+]).
+
+-include_lib("couch_scanner/include/couch_scanner_plugin.hrl").
+
+-record(st, {
+ sid,
+ opts = #{},
+ dbname,
+ report = #{},
+ run_on_first_node = true,
+ doc_report = false
+}).
+
+-define(CONFLICTS, <<"conflicts">>).
+-define(DELETED_CONFLICTS, <<"deleted_conflicts">>).
+
+-define(OPTS, #{
+ ?CONFLICTS => true,
+ ?DELETED_CONFLICTS => false
Review Comment:
```suggestion
?DELETED_CONFLICTS => true
```
as per above
##########
src/couch_scanner/src/couch_scanner_plugin_conflict_finder.erl:
##########
@@ -0,0 +1,196 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_scanner_plugin_conflict_finder).
+-behaviour(couch_scanner_plugin).
+
+-export([
+ start/2,
+ resume/2,
+ complete/1,
+ checkpoint/1,
+ db/2,
+ doc_id/3
+]).
+
+-include_lib("couch_scanner/include/couch_scanner_plugin.hrl").
+
+-record(st, {
+ sid,
+ opts = #{},
+ dbname,
+ report = #{},
+ run_on_first_node = true,
+ doc_report = false
+}).
+
+-define(CONFLICTS, <<"conflicts">>).
+-define(DELETED_CONFLICTS, <<"deleted_conflicts">>).
+
+-define(OPTS, #{
+ ?CONFLICTS => true,
+ ?DELETED_CONFLICTS => false
+}).
+
+% Behavior callbacks
+
+start(SId, #{}) ->
+ St = init_config(#st{sid = SId}),
+ case should_run(St) of
+ true ->
+ ?INFO("Starting.", [], #{sid => SId}),
+ {ok, St};
+ false ->
+ ?INFO("Not starting. Not on first node.", [], #{sid => SId}),
+ skip
+ end.
+
+resume(SId, #{<<"opts">> := OldOpts}) ->
+ St = init_config(#st{sid = SId}),
+ case {OldOpts == St#st.opts, should_run(St)} of
+ {true, true} ->
+ ?INFO("Resuming.", [], #{sid => SId}),
+ {ok, St};
+ {false, true} ->
+ ?INFO("Resetting. Config changed.", [], #{sid => SId}),
+ reset;
+ {_, false} ->
+ ?INFO("Not resuming. Not on first node.", [], #{sid => SId}),
+ skip
+ end.
+
+complete(#st{sid = SId, dbname = DbName, report = Report} = St) ->
+ report_per_db(St, DbName, Report),
+ ?INFO("Completed", [], #{sid => SId}),
+ {ok, #{}}.
+
+checkpoint(#st{sid = SId, opts = Opts}) ->
+ case Opts == opts() of
+ true ->
+ {ok, #{<<"opts">> => Opts}};
+ false ->
+ ?INFO("Resetting. Config changed.", [], #{sid => SId}),
+ reset
+ end.
+
+db(#st{} = St, DbName) ->
+ case St#st.run_on_first_node of
+ true ->
+ {ok, St};
+ false ->
+ % If we run on all nodes spread db checks across nodes
+ case couch_scanner_util:consistent_hash_nodes(DbName) of
+ true -> {ok, St};
+ false -> {skip, St}
+ end
+ end.
+
+doc_id(#st{} = St, <<?DESIGN_DOC_PREFIX, _/binary>>, _Db) ->
+ {skip, St};
+doc_id(#st{} = St, DocId, Db) ->
+ {ok, #doc_info{revs = Revs}} = couch_db:get_doc_info(Db, DocId),
+ DbName = mem3:dbname(couch_db:name(Db)),
+ {ok, check(St, DbName, DocId, Revs)}.
+
+% Private
+
+init_config(#st{} = St) ->
+ St#st{
+ opts = opts(),
+ run_on_first_node = cfg_bool("run_on_first_node",
St#st.run_on_first_node),
+ doc_report = cfg_bool("doc_report", St#st.doc_report)
+ }.
+
+should_run(#st{run_on_first_node = true}) ->
+ couch_scanner_util:on_first_node();
+should_run(#st{run_on_first_node = false}) ->
+ true.
+
+check(#st{} = St, _, _, Revs) when length(Revs) =< 1 ->
+ St;
+check(#st{doc_report = true, opts = Opts} = St, DbName, DocId, Revs) ->
+ {DeletedConflicts, Conflicts} =
+ lists:partition(fun(R) -> R#rev_info.deleted end, Revs),
+ ConflictsReport = gen_report(doc, ?CONFLICTS, Opts, Conflicts),
+ DeletedConflictsReport = gen_report(doc, ?DELETED_CONFLICTS, Opts,
DeletedConflicts),
+ DocReport = maps:merge(ConflictsReport, DeletedConflictsReport),
+ report_per_doc(#st{} = St, DbName, DocId, DocReport),
+ DbReport = maps:from_list([{K, tuple_size(V)} || {K, V} <-
maps:to_list(DocReport)]),
+ report(#st{} = St, DbName, DbReport);
+check(#st{opts = Opts} = St, DbName, _DocId, Revs) ->
+ {DeletedConflicts, Conflicts} =
+ lists:partition(fun(R) -> R#rev_info.deleted end, Revs),
+ ConflictsReport = gen_report(db, ?CONFLICTS, Opts, Conflicts),
+ DeletedConflictsReport = gen_report(db, ?DELETED_CONFLICTS, Opts,
DeletedConflicts),
+ DbReport = maps:merge(ConflictsReport, DeletedConflictsReport),
+ report(#st{} = St, DbName, DbReport).
+
+gen_report(db, ?CONFLICTS, #{?CONFLICTS := true}, Revs) ->
+ #{?CONFLICTS => length(Revs)};
+gen_report(db, ?DELETED_CONFLICTS, #{?DELETED_CONFLICTS := true}, Revs) ->
+ #{?DELETED_CONFLICTS => length(Revs)};
+gen_report(doc, ?CONFLICTS, #{?CONFLICTS := true}, Revs) ->
+ #{?CONFLICTS => list_to_tuple([couch_doc:rev_to_str(R#rev_info.rev) || R
<- Revs])};
+gen_report(doc, ?DELETED_CONFLICTS, #{?DELETED_CONFLICTS := true}, Revs) ->
+ #{?DELETED_CONFLICTS =>
list_to_tuple([couch_doc:rev_to_str(R#rev_info.rev) || R <- Revs])};
+gen_report(_Type, _Key, #{} = _Opts, _Revs) ->
+ #{}.
+
+report(#st{} = St, DbName, Report) ->
+ #st{report = Total, dbname = PrevDbName} = St,
+ case is_binary(PrevDbName) andalso DbName =/= PrevDbName of
+ true ->
+ % We switched dbs, so report stats for old db
+ % and make the new one the current one
+ report_per_db(St, PrevDbName, Total),
+ St#st{report = Report, dbname = DbName};
+ false ->
+ % Keep accumulating per-db stats
+ St#st{report = merge_report(Total, Report), dbname = DbName}
Review Comment:
are we concerned that with heavily conflicted dbs, we accumulate a lot of
memory here?
--
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-unsubscr...@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
