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 <[email protected]> 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),
