(couchdb) branch account-for-compaction-in-node-change-epoch-as-well created (now 1ebc926d7)

[va...@apache.org]

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

vatamane pushed a change to branch account-for-compaction-in-node-change-epoch-as-well
in repository https://gitbox.apache.org/repos/asf/couchdb.git


      at 1ebc926d7 Expect the update sequence in header epochs list to regress

This branch includes the following new commits:

     new 1ebc926d7 Expect the update sequence in header epochs list to regress

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: Expect the update sequence in header epochs list to regress

[va...@apache.org]

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

vatamane pushed a commit to branch account-for-compaction-in-node-change-epoch-as-well
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 1ebc926d728710737e2eaba21c758e7a6c823f33
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Mon Aug 5 11:53:56 2024 -0400

    Expect the update sequence in header epochs list to regress
    
    It can happen during compaction as we figured out in #5163.  In that commit we removed
    the assertion when we stay on the same node. However, it may still be possible
    for someone to resume compaction on another node by copying their files and
    then the assertion could trigger.
    
    Here we remove the assert and add a clause handling the regression. As it's an
    expected condition during compaction so we refuse to update the epochs list.
---
 src/couch/src/couch_bt_engine_header.erl | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/couch/src/couch_bt_engine_header.erl b/src/couch/src/couch_bt_engine_header.erl
index 642ae73ce..3581b1e39 100644
--- a/src/couch/src/couch_bt_engine_header.erl
+++ b/src/couch/src/couch_bt_engine_header.erl
@@ -282,8 +282,13 @@ upgrade_epochs(#db_header{} = Header) ->
                 % compaction proceeds. In that case the epochs sequence could
                 % be regressing.
                 Epochs0;
-            [{_OtherNode, S} | _] = Epochs1 ->
-                assert_monotonic_update_seq(Header, S),
+            [{_OtherNode, S} | _] = Epochs0 when Header#db_header.update_seq < S ->
+                % When compacting the new header starts at update_seq = 0 and
+                % continues incrementing as the compaction proceeds. In that
+                % case the epochs sequence could be regressing. So if we detect
+                % a regression we don't update the epochs list.
+                Epochs0;
+            [{_OtherNode, _S} | _] = Epochs1 ->
                 % This node is taking over ownership of this db
                 % and marking the update sequence where it happened.
                 [{Node, Header#db_header.update_seq} | Epochs1]
@@ -295,15 +300,6 @@ upgrade_epochs(#db_header{} = Header) ->
     DedupedEpochs = remove_dup_epochs(NewEpochs),
     Header#db_header{epochs = DedupedEpochs}.
 
-assert_monotonic_update_seq(#db_header{update_seq = CurrentUpdateSeq}, LatestEpochSeq) ->
-    ?assert(
-        LatestEpochSeq =< CurrentUpdateSeq,
-        "Latest epoch seq " ++
-            integer_to_list(LatestEpochSeq) ++
-            " should not be higher than current update seq " ++
-            integer_to_list(CurrentUpdateSeq)
-    ).
-
 % This is slightly relying on the udpate_seq's being sorted
 % in epochs due to how we only ever push things onto the
 % front. Although if we ever had a case where the update_seq
@@ -460,12 +456,13 @@ upgrade_epochs_test() ->
     CompactionHeader = NowOwnedHeader#db_header{update_seq = 0},
     ?assertEqual(CompactionHeader, upgrade(CompactionHeader)),
 
-    % When nodes are different we do assert sequence regression
+    % When nodes are different, if sequence regression is not recorded as that's
+    % what happens during compaction.we do assert sequence regression
     NotOwnedHighSeq = set(NewHeader, [
         {update_seq, 4},
         {epochs, [{'someothernode@someotherhost', 5}]}
     ]),
-    ?assertError({assert, _}, upgrade(NotOwnedHighSeq)),
+    ?assertEqual(NotOwnedHighSeq, upgrade(NotOwnedHighSeq)),
 
     % Getting a reset header maintains the epoch data
     ResetHeader = from(NewNewHeader),