From a14922fad42575574bb26a24a123e0a1ef4e1621 Mon Sep 17 00:00:00 2001 From: Nick Vatamaniuc Date: Wed, 11 Jan 2023 23:51:05 -0500 Subject: [PATCH 1/6] Ensure design docs are uploaded individually when replicating with _bulk_get Previously, when replication jobs used _bulk_get they didn't upload design docs individually like they do when not using _bulk_get. Here were are preserving an already existing behavior which we had in the replicator without _bulk_get usage for more than 2 years. It was introduced here in #2426. Related to these issues #2415 and #2413. Add tests to cover both attachments and ddoc cases. meck:num_calls/3 is helpful as it allows to nicely assert which API function was called and how many times. --- .../src/couch_replicator_worker.erl | 22 ++++-- .../eunit/couch_replicator_bulk_get_tests.erl | 79 ++++++++++++++++++- 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/src/couch_replicator/src/couch_replicator_worker.erl b/src/couch_replicator/src/couch_replicator_worker.erl index d8f87238892..46e4a6e943c 100644 --- a/src/couch_replicator/src/couch_replicator_worker.erl +++ b/src/couch_replicator/src/couch_replicator_worker.erl @@ -297,17 +297,25 @@ queue_fetch_loop(#fetch_st{} = St) -> {changes, ChangesManager, Changes, ReportSeq} -> % Find missing revisions (POST to _revs_diff) {IdRevs, RdSt1} = find_missing(Changes, Target, Parent, RdSt), - {Docs, BgSt1} = bulk_get(UseBulkGet, Source, IdRevs, Parent, BgSt), - % Documents without attachments can be uploaded right away - BatchFun = fun({_, #doc{} = Doc}) -> - ok = gen_server:call(Parent, {batch_doc, Doc}, infinity) + % Filter out and handle design docs individually + DDocFilter = fun + ({<>, _Rev}, _PAs) -> true; + ({_Id, _Rev}, _PAs) -> false end, - lists:foreach(BatchFun, lists:sort(maps:to_list(Docs))), - % Fetch individually if _bulk_get failed or there are attachments + DDocIdRevs = maps:filter(DDocFilter, IdRevs), FetchFun = fun({Id, Rev}, PAs) -> ok = gen_server:call(Parent, {fetch_doc, {Id, [Rev], PAs}}, infinity) end, - maps:map(FetchFun, maps:without(maps:keys(Docs), IdRevs)), + maps:map(FetchFun, DDocIdRevs), + % IdRevs1 is all the docs without design docs. Bulk get those. + IdRevs1 = maps:without(maps:keys(DDocIdRevs), IdRevs), + {Docs, BgSt1} = bulk_get(UseBulkGet, Source, IdRevs1, Parent, BgSt), + BatchFun = fun({_, #doc{} = Doc}) -> + ok = gen_server:call(Parent, {batch_doc, Doc}, infinity) + end, + lists:foreach(BatchFun, lists:sort(maps:to_list(Docs))), + % Invidually upload docs with attachments. + maps:map(FetchFun, maps:without(maps:keys(Docs), IdRevs1)), {ok, Stats} = gen_server:call(Parent, flush, infinity), ok = report_seq_done(Cp, ReportSeq, Stats), couch_log:debug("Worker reported completion of seq ~p", [ReportSeq]), diff --git a/src/couch_replicator/test/eunit/couch_replicator_bulk_get_tests.erl b/src/couch_replicator/test/eunit/couch_replicator_bulk_get_tests.erl index 2ecd0f4ee91..f0d9569db43 100644 --- a/src/couch_replicator/test/eunit/couch_replicator_bulk_get_tests.erl +++ b/src/couch_replicator/test/eunit/couch_replicator_bulk_get_tests.erl @@ -26,7 +26,11 @@ bulk_get_test_() -> fun couch_replicator_test_helper:test_teardown/1, [ ?TDEF_FE(use_bulk_get), + ?TDEF_FE(use_bulk_get_with_ddocs), + ?TDEF_FE(use_bulk_get_with_attachments), ?TDEF_FE(dont_use_bulk_get), + ?TDEF_FE(dont_use_bulk_get_ddocs), + ?TDEF_FE(dont_use_bulk_get_attachments), ?TDEF_FE(job_enable_overrides_global_disable), ?TDEF_FE(global_disable_works) ] @@ -39,7 +43,33 @@ use_bulk_get({_Ctx, {Source, Target}}) -> replicate(Source, Target, true), BulkGets = meck:num_calls(couch_replicator_api_wrap, bulk_get, 3), JustGets = meck:num_calls(couch_replicator_api_wrap, open_doc_revs, 6), + DocUpdates = meck:num_calls(couch_replicator_api_wrap, update_doc, 4), ?assertEqual(0, JustGets), + ?assertEqual(0, DocUpdates), + ?assert(BulkGets >= 1), + compare_dbs(Source, Target). + +use_bulk_get_with_ddocs({_Ctx, {Source, Target}}) -> + populate_db_ddocs(Source, ?DOC_COUNT), + meck:new(couch_replicator_api_wrap, [passthrough]), + replicate(Source, Target, true), + BulkGets = meck:num_calls(couch_replicator_api_wrap, bulk_get, 3), + JustGets = meck:num_calls(couch_replicator_api_wrap, open_doc_revs, 6), + DocUpdates = meck:num_calls(couch_replicator_api_wrap, update_doc, 4), + ?assertEqual(?DOC_COUNT, JustGets), + ?assertEqual(?DOC_COUNT, DocUpdates), + ?assert(BulkGets >= 1), + compare_dbs(Source, Target). + +use_bulk_get_with_attachments({_Ctx, {Source, Target}}) -> + populate_db_atts(Source, ?DOC_COUNT), + meck:new(couch_replicator_api_wrap, [passthrough]), + replicate(Source, Target, true), + BulkGets = meck:num_calls(couch_replicator_api_wrap, bulk_get, 3), + JustGets = meck:num_calls(couch_replicator_api_wrap, open_doc_revs, 6), + DocUpdates = meck:num_calls(couch_replicator_api_wrap, update_doc, 4), + ?assertEqual(?DOC_COUNT, JustGets), + ?assertEqual(?DOC_COUNT, DocUpdates), ?assert(BulkGets >= 1), compare_dbs(Source, Target). @@ -49,10 +79,36 @@ dont_use_bulk_get({_Ctx, {Source, Target}}) -> replicate(Source, Target, false), BulkGets = meck:num_calls(couch_replicator_api_wrap, bulk_get, 3), JustGets = meck:num_calls(couch_replicator_api_wrap, open_doc_revs, 6), + DocUpdates = meck:num_calls(couch_replicator_api_wrap, update_doc, 4), ?assertEqual(0, BulkGets), + ?assertEqual(0, DocUpdates), ?assertEqual(?DOC_COUNT, JustGets), compare_dbs(Source, Target). +dont_use_bulk_get_ddocs({_Ctx, {Source, Target}}) -> + populate_db_ddocs(Source, ?DOC_COUNT), + meck:new(couch_replicator_api_wrap, [passthrough]), + replicate(Source, Target, false), + BulkGets = meck:num_calls(couch_replicator_api_wrap, bulk_get, 3), + JustGets = meck:num_calls(couch_replicator_api_wrap, open_doc_revs, 6), + DocUpdates = meck:num_calls(couch_replicator_api_wrap, update_doc, 4), + ?assertEqual(0, BulkGets), + ?assertEqual(?DOC_COUNT, JustGets), + ?assertEqual(?DOC_COUNT, DocUpdates), + compare_dbs(Source, Target). + +dont_use_bulk_get_attachments({_Ctx, {Source, Target}}) -> + populate_db_atts(Source, ?DOC_COUNT), + meck:new(couch_replicator_api_wrap, [passthrough]), + replicate(Source, Target, false), + BulkGets = meck:num_calls(couch_replicator_api_wrap, bulk_get, 3), + JustGets = meck:num_calls(couch_replicator_api_wrap, open_doc_revs, 6), + DocUpdates = meck:num_calls(couch_replicator_api_wrap, update_doc, 4), + ?assertEqual(0, BulkGets), + ?assertEqual(?DOC_COUNT, JustGets), + ?assertEqual(?DOC_COUNT, DocUpdates), + compare_dbs(Source, Target). + job_enable_overrides_global_disable({_Ctx, {Source, Target}}) -> populate_db(Source, ?DOC_COUNT), Persist = false, @@ -78,10 +134,31 @@ global_disable_works({_Ctx, {Source, Target}}) -> compare_dbs(Source, Target). populate_db(DbName, DocCount) -> - Fun = fun(Id, Acc) -> [#doc{id = integer_to_binary(Id)} | Acc] end, + IdFun = fun(Id) -> integer_to_binary(Id) end, + Fun = fun(Id, Acc) -> [#doc{id = IdFun(Id)} | Acc] end, + Docs = lists:foldl(Fun, [], lists:seq(1, DocCount)), + {ok, _} = fabric:update_docs(DbName, Docs, [?ADMIN_CTX]). + +populate_db_ddocs(DbName, DocCount) -> + IdFun = fun(Id) -> <<"_design/", (integer_to_binary(Id))/binary>> end, + Fun = fun(Id, Acc) -> [#doc{id = IdFun(Id)} | Acc] end, Docs = lists:foldl(Fun, [], lists:seq(1, DocCount)), {ok, _} = fabric:update_docs(DbName, Docs, [?ADMIN_CTX]). +populate_db_atts(DbName, DocCount) -> + IdFun = fun(Id) -> integer_to_binary(Id) end, + Fun = fun(Id, Acc) -> [#doc{id = IdFun(Id), atts = [att(<<"a">>)]} | Acc] end, + Docs = lists:foldl(Fun, [], lists:seq(1, DocCount)), + {ok, _} = fabric:update_docs(DbName, Docs, [?ADMIN_CTX]). + +att(Name) when is_binary(Name) -> + couch_att:new([ + {name, Name}, + {att_len, 1}, + {type, <<"app/binary">>}, + {data, <<"x">>} + ]). + compare_dbs(Source, Target) -> couch_replicator_test_helper:cluster_compare_dbs(Source, Target). From dad51b5b82e37838e030cb4a7f4abbd90f6c2403 Mon Sep 17 00:00:00 2001 From: Gabor Pali Date: Tue, 17 Jan 2023 23:30:47 +0100 Subject: [PATCH 2/6] Set the SpiderMonkey version for the first phase of PR builds --- build-aux/Jenkinsfile.pr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-aux/Jenkinsfile.pr b/build-aux/Jenkinsfile.pr index bcb8b69ad7f..cf97c900f8c 100644 --- a/build-aux/Jenkinsfile.pr +++ b/build-aux/Jenkinsfile.pr @@ -183,7 +183,7 @@ pipeline { steps { sh ''' rm -rf apache-couchdb-* - ./configure --skip-deps + ./configure --skip-deps --spidermonkey-version 78 make erlfmt-check make elixir-source-checks make python-black From aff7a6e7183e6c471a9675e37216baa0bad563a3 Mon Sep 17 00:00:00 2001 From: Gabor Pali Date: Tue, 17 Jan 2023 10:54:19 +0100 Subject: [PATCH 3/6] docs(mango): match description of `$mod` with reality The remainder argument for the `$mod` operator can be zero, while its documentation suggests otherwise. It actually covers a very realistic use case where divisibility is expressed. Neither related restrictions could be identified in the sources [1] nor MongoDB forbids this [2]. Tests also seem to exercise this specific case [3]. Thanks @iilyak for checking on these. [1] https://github.com/apache/couchdb/blob/adf17140e81d0b74f2b2ecdea48fc4f702832eaf/src/mango/src/mango_selector.erl#L512:L513 [2] https://www.mongodb.com/docs/manual/reference/operator/query/mod/ [3] https://github.com/apache/couchdb/blob/0059b8f90e58e10b199a4b768a06a762d12a30d3/src/mango/test/03-operator-test.py#L58 --- src/docs/src/api/database/find.rst | 4 ++-- src/mango/README.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/docs/src/api/database/find.rst b/src/docs/src/api/database/find.rst index b5a0bd4601c..0e511e6ad96 100644 --- a/src/docs/src/api/database/find.rst +++ b/src/docs/src/api/database/find.rst @@ -705,8 +705,8 @@ operators require the argument to be in a specific JSON format. | | | | document. Non-array fields cannot | | | | | match this condition. | +---------------+-------------+------------+-----------------------------------+ -| Miscellaneous | ``$mod`` | [Divisor, | Divisor and Remainder are both | -| | | Remainder] | positive or negative integers. | +| Miscellaneous | ``$mod`` | [Divisor, | Divisor is a non-zero integer, | +| | | Remainder] | Remainder is any integer. | | | | | Non-integer values result in a | | | | | 404. Matches documents where | | | | | ``field % Divisor == Remainder`` | diff --git a/src/mango/README.md b/src/mango/README.md index 4c4bb60a672..9fa273b1570 100644 --- a/src/mango/README.md +++ b/src/mango/README.md @@ -302,7 +302,7 @@ Array related operators Misc related operators -* "$mod" - [Divisor, Remainder], where Divisor and Remainder are both positive integers (ie, greater than 0). Matches documents where (field % Divisor == Remainder) is true. This is false for any non-integer field +* "$mod" - [Divisor, Remainder], where Divisor is a non-zero integer and Remainder is any integer. Matches documents where (field % Divisor == Remainder) is true. This is false for any non-integer field * "$regex" - string, a regular expression pattern to match against the document field. Only matches when the field is a string value and matches the supplied matches From deb8ef2714f4bd98b3f0e85e5573cc14a58ab8fb Mon Sep 17 00:00:00 2001 From: Ronny Berndt Date: Wed, 18 Jan 2023 18:36:41 +0100 Subject: [PATCH 4/6] Adding build-report makefile target on Windows (#4384) Backporting the `build-report` target from the *nix makefile to the Windows pendant. --- Makefile.win | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Makefile.win b/Makefile.win index a897554e7a7..13b83f7810a 100644 --- a/Makefile.win +++ b/Makefile.win @@ -250,6 +250,11 @@ elixir-check-formatted: elixir-init elixir-credo: elixir-init @mix credo +.PHONY: build-report +# target: build-report - Generate a build report +build-report: + @$(PYTHON) build-aux/show-test-results.py --suites=10 --tests=10 > test-results.log + .PHONY: check-qs # target: check-qs - Run query server tests (ruby and rspec required!) check-qs: From 4b2d3dc6253c821c443bc1e1b4540cf76d848908 Mon Sep 17 00:00:00 2001 From: Nick Vatamaniuc Date: Wed, 18 Jan 2023 11:08:55 -0500 Subject: [PATCH 5/6] Bump recon to 2.5.3 Changes since 2.5.2: https://github.com/ferd/recon/compare/2.5.2...2.5.3 --- rebar.config.script | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rebar.config.script b/rebar.config.script index 028aabe8f18..2bdb17a49a7 100644 --- a/rebar.config.script +++ b/rebar.config.script @@ -156,7 +156,7 @@ DepDescs = [ {jiffy, "jiffy", {tag, "CouchDB-1.0.9-2"}}, {mochiweb, "mochiweb", {tag, "v3.1.1"}}, {meck, "meck", {tag, "0.9.2"}}, -{recon, "recon", {tag, "2.5.2"}} +{recon, "recon", {tag, "2.5.3"}} ]. WithProper = lists:keyfind(with_proper, 1, CouchConfig) == {with_proper, true}. From 3b6ebdeb3b5d10f03677b5f2c38c21c6c144a2bd Mon Sep 17 00:00:00 2001 From: Jan Lehnardt Date: Wed, 18 Jan 2023 16:01:58 +0100 Subject: [PATCH 6/6] ci(mac): re-enable mac CI for the full build --- build-aux/Jenkinsfile.full | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/build-aux/Jenkinsfile.full b/build-aux/Jenkinsfile.full index efbf1db5af3..bce9f14eca3 100644 --- a/build-aux/Jenkinsfile.full +++ b/build-aux/Jenkinsfile.full @@ -86,12 +86,11 @@ meta = [ // gnu_make: 'gmake' // ], - /// Temporarily bypass macos builder due to rebar version issue - // 'macos': [ - // name: 'macOS', - // spidermonkey_vsn: '60', - // gnu_make: 'make' - // ] + 'macos': [ + name: 'macOS', + spidermonkey_vsn: '91', + gnu_make: 'make' + ] ] // Credit to https://stackoverflow.com/a/69222555 for this technique.