You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2022/09/15 20:47:39 UTC

[couchdb] branch remove-long-deprecated-bigcouch-sequences updated (c03d6d314 -> f4cc19080)

This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a change to branch remove-long-deprecated-bigcouch-sequences
in repository https://gitbox.apache.org/repos/asf/couchdb.git


 discard c03d6d314 Remove the long deprecated bigcouch 0.4 change sequence support
     new f4cc19080 Remove the long deprecated bigcouch 0.4 change sequence support

This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version.  This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:

 * -- * -- B -- O -- O -- O   (c03d6d314)
            \
             N -- N -- N   refs/heads/remove-long-deprecated-bigcouch-sequences (f4cc19080)

You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.

Any revisions marked "omit" are not gone; other references still
refer to them.  Any revisions marked "discard" are gone forever.

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:


[couchdb] 01/01: Remove the long deprecated bigcouch 0.4 change sequence support

Posted by va...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a commit to branch remove-long-deprecated-bigcouch-sequences
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit f4cc190806967eee2c22e5625b0e87e5f84dbb0e
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Thu Sep 15 16:34:22 2022 -0400

    Remove the long deprecated bigcouch 0.4 change sequence support
    
    We always ran a regex match for it, then failed, and tried the regular pattern.
    Regexes are pretty fast (15-30 microseconds on my laptop), but it's still a
    waste of resources. For comparison a `config:get/3` is about microsecond, so
    it's like needlessly performing 10 extra config lookup per `_changes` feed
    request.
    
    Improve unit tests by having more cases and one test per case.
    
    Remove unreachable `"now"` clause in `validate_start_seq/2`. By that point
    `get_start_seq/2` should have transformed `"now"` into a proper sequence so
    it's confusing validating that case as passing. At that point it should be
    considered a failure, which is what one of the new unit tests checks for.
---
 src/fabric/src/fabric_view_changes.erl | 192 +++++++++++++++++++--------------
 1 file changed, 110 insertions(+), 82 deletions(-)

diff --git a/src/fabric/src/fabric_view_changes.erl b/src/fabric/src/fabric_view_changes.erl
index b561da151..0ee996df7 100644
--- a/src/fabric/src/fabric_view_changes.erl
+++ b/src/fabric/src/fabric_view_changes.erl
@@ -24,7 +24,6 @@
 -include_lib("fabric/include/fabric.hrl").
 -include_lib("mem3/include/mem3.hrl").
 -include_lib("couch/include/couch_db.hrl").
--include_lib("eunit/include/eunit.hrl").
 
 -import(fabric_db_update_listener, [wait_db_updated/1]).
 
@@ -431,16 +430,10 @@ seq({Seq, _Uuid}) -> Seq;
 seq(Seq) -> Seq.
 
 unpack_seq_regex_match(Packed) ->
-    NewPattern = "^\\[[0-9]+\s*,\s*\"(?<opaque>.*)\"\\]$",
-    OldPattern = "^\"?([0-9]+-)?(?<opaque>.*?)\"?$",
+    Pattern = "^\"?([0-9]+-)?(?<opaque>.*?)\"?$",
     Options = [{capture, [opaque], binary}],
-    case re:run(Packed, NewPattern, Options) of
-        {match, Match} ->
-            Match;
-        nomatch ->
-            {match, Match} = re:run(Packed, OldPattern, Options),
-            Match
-    end.
+    {match, Match} = re:run(Packed, Pattern, Options),
+    Match.
 
 unpack_seq_decode_term(Opaque) ->
     binary_to_term(couch_util:decodeBase64Url(Opaque)).
@@ -464,9 +457,6 @@ unpack_seqs(0, DbName) ->
     fabric_dict:init(mem3:shards(DbName), 0);
 unpack_seqs("0", DbName) ->
     fabric_dict:init(mem3:shards(DbName), 0);
-% deprecated
-unpack_seqs([_SeqNum, Opaque], DbName) ->
-    do_unpack_seqs(Opaque, DbName);
 unpack_seqs(Packed, DbName) ->
     Opaque = unpack_seq_regex_match(Packed),
     do_unpack_seqs(Opaque, DbName).
@@ -758,21 +748,14 @@ make_split_seq({Num, Uuid, Node}, RepCount) when RepCount > 1 ->
 make_split_seq(Seq, _) ->
     Seq.
 
-validate_start_seq(_DbName, "now") ->
-    ok;
 validate_start_seq(_DbName, 0) ->
     ok;
 validate_start_seq(_DbName, "0") ->
     ok;
-validate_start_seq(_DbName, Seq) ->
+validate_start_seq(_DbName, Seq) when is_list(Seq) orelse is_binary(Seq) ->
     try
-        case Seq of
-            [_SeqNum, Opaque] ->
-                unpack_seq_decode_term(Opaque);
-            Seq ->
-                Opaque = unpack_seq_regex_match(Seq),
-                unpack_seq_decode_term(Opaque)
-        end,
+        Opaque = unpack_seq_regex_match(Seq),
+        unpack_seq_decode_term(Opaque),
         ok
     catch
         _:_ ->
@@ -792,10 +775,17 @@ get_changes_epoch() ->
 increment_changes_epoch() ->
     application:set_env(fabric, changes_epoch, os:timestamp()).
 
+-ifdef(TEST).
+
+-include_lib("eunit/include/eunit.hrl").
+
+-define(TDEF(Name), {atom_to_list(Name), fun Name/0}).
+
 unpack_seq_setup() ->
     meck:new(mem3),
     meck:new(fabric_view),
     meck:expect(mem3, get_shard, fun(_, _, _) -> {ok, #shard{}} end),
+    meck:expect(mem3, shards, fun(_) -> [#shard{}] end),
     meck:expect(fabric_ring, is_progress_possible, fun(_) -> true end),
     ok.
 
@@ -805,75 +795,111 @@ unpack_seqs_test_() ->
         fun unpack_seq_setup/0,
         fun(_) -> meck:unload() end,
         [
-            t_unpack_seqs()
+            ?TDEF(t_full_seq),
+            ?TDEF(t_full_seq_quoted),
+            ?TDEF(t_full_seq_with_hyphen),
+            ?TDEF(t_full_seq_quoted_with_hyphen),
+            ?TDEF(t_no_numeric_prefix),
+            ?TDEF(t_zero_seq_int),
+            ?TDEF(t_zero_seq_string),
+            ?TDEF(t_fail_now),
+            ?TDEF(t_fail_numeric_int),
+            ?TDEF(t_fail_numeric_string),
+            ?TDEF(t_fail_numeric_string_quoted),
+            ?TDEF(t_fail_empty_string),
+            ?TDEF(t_fail_empty_string_quoted),
+            ?TDEF(t_fail_random_junk),
+            ?TDEF(t_fail_old_bigcouch_term),
+            ?TDEF(t_fail_old_bigcouch_string)
         ]
     }.
 
-t_unpack_seqs() ->
-    ?_test(begin
-        % BigCouch 0.3 style.
-        assert_shards(
-            "23423-g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
-            "LXMwBCwxygOFMiQ5L8____sxIZcKlIUgCSSfZgRUw4FTmAFMWDFTHiVJQAUlSPX1Ee"
-            "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA"
-        ),
+t_full_seq() ->
+    assert_shards(
+        "23423-g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
+        "LXMwBCwxygOFMiQ5L8____sxIZcKlIUgCSSfZgRUw4FTmAFMWDFTHiVJQAUlSPX1Ee"
+        "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA"
+    ).
 
-        % BigCouch 0.4 style.
-        assert_shards([
-            23423,
-            <<
-                "g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
-                "LXMwBCwxygOFMiQ5L8____sxIZcKlIUgCSSfZgRUw4FTmAFMWDFTHiVJQAUlSPX1Ee"
-                "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA"
-            >>
-        ]),
-
-        % BigCouch 0.4 style (as string).
-        assert_shards(
-            "[23423,\"g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
-            "LXMwBCwxygOFMiQ5L8____sxIZcKlIUgCSSfZgRUw4FTmAFMWDFTHiVJQAUlSPX1Ee"
-            "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA\"]"
-        ),
-        assert_shards(
-            "[23423 ,\"g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
-            "LXMwBCwxygOFMiQ5L8____sxIZcKlIUgCSSfZgRUw4FTmAFMWDFTHiVJQAUlSPX1Ee"
-            "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA\"]"
-        ),
-        assert_shards(
-            "[23423, \"g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
-            "LXMwBCwxygOFMiQ5L8____sxIZcKlIUgCSSfZgRUw4FTmAFMWDFTHiVJQAUlSPX1Ee"
-            "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA\"]"
-        ),
-        assert_shards(
-            "[23423 , \"g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
-            "LXMwBCwxygOFMiQ5L8____sxIZcKlIUgCSSfZgRUw4FTmAFMWDFTHiVJQAUlSPX1Ee"
-            "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA\"]"
-        ),
+t_full_seq_quoted() ->
+    assert_shards(
+        "\"23423-g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
+        "LXMwBCwxygOFMiQ5L8____sxIZcKlIUgCSSfZgRUw4FTmAFMWDFTHiVJQAUlSPX1Ee"
+        "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA\""
+    ).
 
-        % with internal hypen
-        assert_shards(
-            "651-g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwNDLXMwBCwxygOFMiQ"
-            "5L8____sxJTcalIUgCSSfZgReE4FTmAFMWDFYXgVJQAUlQPVuSKS1EeC5BkaABSQHXz8"
-            "VgJUbgAonB_VqIPfoUHIArvE7T6AUQh0I1-WQAzp1XB"
-        ),
-        assert_shards([
-            651,
-            "g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwNDLXMwBCwxygOFMiQ"
-            "5L8____sxJTcalIUgCSSfZgReE4FTmAFMWDFYXgVJQAUlQPVuSKS1EeC5BkaABSQHXz8"
-            "VgJUbgAonB_VqIPfoUHIArvE7T6AUQh0I1-WQAzp1XB"
-        ]),
-
-        % CouchDB 1.2 style
-        assert_shards(
-            "\"23423-g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
+t_full_seq_with_hyphen() ->
+    assert_shards(
+        "651-g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwNDLXMwBCwxygOFMiQ"
+        "5L8____sxJTcalIUgCSSfZgReE4FTmAFMWDFYXgVJQAUlQPVuSKS1EeC5BkaABSQHXz8"
+        "VgJUbgAonB_VqIPfoUHIArvE7T6AUQh0I1-WQAzp1XB"
+    ).
+
+t_full_seq_quoted_with_hyphen() ->
+    assert_shards(
+        "\"651-g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwNDLXMwBCwxygOFMiQ"
+        "5L8____sxJTcalIUgCSSfZgReE4FTmAFMWDFYXgVJQAUlQPVuSKS1EeC5BkaABSQHXz8"
+        "VgJUbgAonB_VqIPfoUHIArvE7T6AUQh0I1-WQAzp1XB\""
+    ).
+
+t_no_numeric_prefix() ->
+    assert_shards(
+        "g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
+        "LXMwBCwxygOFMiQ5L8____sxIZcKlIUgCSSfZgRUw4FTmAFMWDFTHiVJQAUlSPX1Ee"
+        "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA"
+    ).
+
+t_zero_seq_int() ->
+    assert_shards(0).
+
+t_zero_seq_string() ->
+    assert_shards("0").
+
+t_fail_now() ->
+    % "now" should have been transformed into a sequence in get_start_seq/2
+    fail("now").
+
+t_fail_numeric_int() ->
+    fail(1234).
+
+t_fail_numeric_string() ->
+    fail("1234").
+
+t_fail_numeric_string_quoted() ->
+    fail("\"1234\"").
+
+t_fail_empty_string() ->
+    fail("").
+
+t_fail_empty_string_quoted() ->
+    fail("\"\"").
+
+t_fail_random_junk() ->
+    fail("randomjunk").
+
+t_fail_old_bigcouch_term() ->
+    fail([
+        23423,
+        <<
+            "g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
             "LXMwBCwxygOFMiQ5L8____sxIZcKlIUgCSSfZgRUw4FTmAFMWDFTHiVJQAUlSPX1Ee"
-            "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA\""
-        )
-    end).
+            "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA"
+        >>
+    ]).
+
+t_fail_old_bigcouch_string() ->
+    fail(
+        "[23423,\"g1AAAAE7eJzLYWBg4MhgTmHgS0ktM3QwND"
+        "LXMwBCwxygOFMiQ5L8____sxIZcKlIUgCSSfZgRUw4FTmAFMWDFTHiVJQAUlSPX1Ee"
+        "C5BkaABSQHXzsxKZ8StcAFG4H4_bIAoPQBTeJ2j1A4hCUJBkAQC7U1NA\"]"
+    ).
 
 assert_shards(Packed) ->
     ?assertMatch([{#shard{}, _} | _], unpack_seqs(Packed, <<"foo">>)).
 
+fail(Packed) ->
+    ?assertError(badarg, unpack_seqs(Packed, <<"foo">>)).
+
 find_replacements_test() ->
     % None of the workers are in the live list of shard but there is a
     % replacement on n3 for the full range. It should get picked instead of
@@ -1040,3 +1066,5 @@ find_replacement_sequence_test() ->
     Dead4 = mk_workers(Shards, {replace, 'n1', Uuid, 45}),
     ?assertEqual(0, find_replacement_sequence(Dead4, [0, 10])),
     ?assertEqual(0, find_replacement_sequence(Dead4, [0, 5])).
+
+-endif.