[couchdb] branch prettify-split-sequences created (now ead9f67aa)

[va...@apache.org]

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

vatamane pushed a change to branch prettify-split-sequences
in repository https://gitbox.apache.org/repos/asf/couchdb.git


      at ead9f67aa Improve emitted change feed sequence after a split

This branch includes the following new commits:

     new ead9f67aa Improve emitted change feed sequence after a split

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.

[couchdb] 01/01: Improve emitted change feed sequence after a split

[va...@apache.org]

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

vatamane pushed a commit to branch prettify-split-sequences
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit ead9f67aa60de1f89d5958420956d9309269d525
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Thu Jun 15 20:32:46 2023 -0400

    Improve emitted change feed sequence after a split
    
     When we get sequence before the split, we'll fill in the missing (now split)
    ranges with a special `{split, OldNodeUUid}` marker. However, when sequences
    are emitted in the changes API, that will make the N- prefix (SeqSum) bounce
    around from higher to lower numbers, while users expect those to be mostly
    incrementing. So take a conservative approach and assume it will be full
    rewind for that ramge, and use 0 for that range. This is a purely cosmetic
    thing, when we decode the sequence that prefix gets thrown away anyway.
    
    Fixes a part of issue: #4640
---
 src/fabric/src/fabric_view_changes.erl | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/fabric/src/fabric_view_changes.erl b/src/fabric/src/fabric_view_changes.erl
index fd439c5c3..84046d174 100644
--- a/src/fabric/src/fabric_view_changes.erl
+++ b/src/fabric/src/fabric_view_changes.erl
@@ -425,6 +425,15 @@ pack_seqs(Workers) ->
     Opaque = couch_util:encodeBase64Url(?term_to_bin(SeqList, [compressed])),
     ?l2b([integer_to_list(SeqSum), $-, Opaque]).
 
+% When we get sequence before the split, we'll fill in the missing (now split)
+% ranges with a special {split, OldNodeUUid} marker. However, when sequences
+% are emitted in the changes API, that will make the N- prefix (SeqSum) bounce
+% around from higher to lower numbers, while users expect those to be mostly
+% incrementing. So take a conservative approach and assume it will be full
+% rewind for that ramge, and use 0 for that range. This is a purely cosmetic
+% thing, when we decode the sequence that prefix gets thrown away anyway.
+%
+seq({Seq, {split, <<_/binary>>}, _Node}) when is_integer(Seq) -> 0;
 seq({Seq, _Uuid, _Node}) -> Seq;
 seq({Seq, _Uuid}) -> Seq;
 seq(Seq) -> Seq.
@@ -1065,4 +1074,18 @@ find_replacement_sequence_test() ->
     ?assertEqual(0, find_replacement_sequence(Dead4, [0, 10])),
     ?assertEqual(0, find_replacement_sequence(Dead4, [0, 5])).
 
+pack_split_seq_test() ->
+    Shards = [{"n1", 0, 10}, {"n2", 11, ?RING_END}],
+    Workers = mk_workers(Shards, {42, {split, <<"abc1234">>}, 'node1@127.0.0.1'}),
+    PackedSeq = pack_seqs(Workers),
+    ?assertMatch(<<"0-", _/binary>>, PackedSeq),
+    DecodedSeq = decode_seq(PackedSeq),
+    ?assertEqual(
+        [
+            {n1, [0, 10], {42, {split, <<"abc1234">>}, 'node1@127.0.0.1'}},
+            {n2, [11, 4294967295], {42, {split, <<"abc1234">>}, 'node1@127.0.0.1'}}
+        ],
+        DecodedSeq
+    ).
+
 -endif.