This is an automated email from the ASF dual-hosted git repository. vatamane pushed a commit to branch merge-3.4.3 in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 8c5f186d2d22d178f562c7a15abcb42cbc308ae4 Author: Nick Vatamaniuc <[email protected]> AuthorDate: Tue Mar 4 16:41:26 2025 -0500 Fix compatibility clause for attachments In a mixed cluster, with nodes before and after #5373 where `pread_iolist` and `append_bin` were removed, attachment operations may throw function_clause errors on the new couch_file gen_servers. That's because attachment streams may be opened on one node, but operations on them maybe called from another node. As such, we have to handle the old gen_server messages until the next version. --- src/couch/src/couch_file.erl | 54 ++++++++++++++++++++++--------- src/couch/test/eunit/couch_file_tests.erl | 43 +++++++++++++++++++++++- 2 files changed, 80 insertions(+), 17 deletions(-) diff --git a/src/couch/src/couch_file.erl b/src/couch/src/couch_file.erl index 78189c64e..fcccd12ed 100644 --- a/src/couch/src/couch_file.erl +++ b/src/couch/src/couch_file.erl @@ -529,6 +529,16 @@ terminate(_Reason, #file{fd = Fd}) -> handle_call(close, _From, #file{fd = Fd} = File) -> {stop, normal, file:close(Fd), File#file{fd = nil}}; +handle_call({pread_iolist, Pos}, _From, File) -> + % Compatibility clause. Remove after upgrading to next release after 3.5.0 + % Attachment streams may be opened from remote nodes during the upgrade on + % a mixed cluster + Result = + case pread(File, [Pos]) of + {ok, [{IoList, Checksum}]} -> {ok, IoList, Checksum}; + Error -> Error + end, + {reply, Result, File}; handle_call({pread_iolists, PosL}, _From, File) -> {reply, pread(File, PosL), File}; handle_call(bytes, _From, #file{} = File) -> @@ -559,22 +569,18 @@ handle_call({truncate, Pos}, _From, #file{fd = Fd} = File) -> Error -> {reply, Error, File} end; -handle_call({append_bins, Bins}, _From, #file{fd = Fd, eof = Pos} = File) -> - {BlockResps, FinalPos} = lists:mapfoldl( - fun(Bin, PosAcc) -> - Blocks = make_blocks(PosAcc rem ?SIZE_BLOCK, Bin), - Size = iolist_size(Blocks), - {{Blocks, {PosAcc, Size}}, PosAcc + Size} - end, - Pos, - Bins - ), - {AllBlocks, Resps} = lists:unzip(BlockResps), - case file:write(Fd, AllBlocks) of - ok -> - {reply, {ok, Resps}, File#file{eof = FinalPos}}; - Error -> - {reply, Error, reset_eof(File)} +handle_call({append_bin, Bin}, _From, #file{} = File) -> + % Compatibility clause. Remove after upgrading to next release after 3.5.0 + % Attachment streams may be opened from remote nodes during the upgrade on + % a mixed cluster + case append_bins(File, [Bin]) of + {{ok, [{Pos, Len}]}, File1} -> {reply, {ok, Pos, Len}, File1}; + {Error, File1} -> {reply, Error, File1} + end; +handle_call({append_bins, Bins}, _From, #file{} = File) -> + case append_bins(File, Bins) of + {{ok, Resps}, File1} -> {reply, {ok, Resps}, File1}; + {Error, File1} -> {reply, Error, File1} end; handle_call({write_header, Bin}, _From, #file{fd = Fd, eof = Pos} = File) -> BinSize = byte_size(Bin), @@ -618,6 +624,22 @@ format_status(_Opt, [PDict, #file{} = File]) -> eof(#file{fd = Fd}) -> file:position(Fd, eof). +append_bins(#file{fd = Fd, eof = Pos} = File, Bins) -> + {BlockResps, FinalPos} = lists:mapfoldl( + fun(Bin, PosAcc) -> + Blocks = make_blocks(PosAcc rem ?SIZE_BLOCK, Bin), + Size = iolist_size(Blocks), + {{Blocks, {PosAcc, Size}}, PosAcc + Size} + end, + Pos, + Bins + ), + {AllBlocks, Resps} = lists:unzip(BlockResps), + case file:write(Fd, AllBlocks) of + ok -> {{ok, Resps}, File#file{eof = FinalPos}}; + Error -> {Error, reset_eof(File)} + end. + pread(#file{} = File, PosL) -> LocNums1 = [{Pos, 4} || Pos <- PosL], DataSizes = read_multi_raw_iolists_int(File, LocNums1), diff --git a/src/couch/test/eunit/couch_file_tests.erl b/src/couch/test/eunit/couch_file_tests.erl index 4397adf0c..5b7481636 100644 --- a/src/couch/test/eunit/couch_file_tests.erl +++ b/src/couch/test/eunit/couch_file_tests.erl @@ -155,7 +155,9 @@ read_write_test_() -> ?TDEF_FE(should_apply_overwrite_create_option), ?TDEF_FE(should_error_on_creation_if_exists), ?TDEF_FE(should_close_on_idle), - ?TDEF_FE(should_crash_on_unexpected_cast) + ?TDEF_FE(should_crash_on_unexpected_cast), + ?TDEF_FE(should_handle_pread_iolist_upgrade_clause), + ?TDEF_FE(should_handle_append_bin_upgrade_clause) ] } } @@ -360,6 +362,45 @@ should_crash_on_unexpected_cast(Fd) -> ?assertEqual({invalid_cast, potato}, Reason) end. +% Remove this test when removing pread_iolist compatibility clause for online +% upgrades from couch_file +% +should_handle_pread_iolist_upgrade_clause(Fd) -> + Bin = <<"bin">>, + {ok, Pos, _} = couch_file:append_binary(Fd, Bin), + ResList = gen_server:call(Fd, {pread_iolists, [Pos]}, infinity), + ?assertMatch({ok, [{_, _}]}, ResList), + {ok, [{IoList1, Checksum1}]} = ResList, + ?assertEqual(Bin, iolist_to_binary(IoList1)), + ?assertEqual(<<>>, Checksum1), + ResSingle = gen_server:call(Fd, {pread_iolist, Pos}, infinity), + ?assertMatch({ok, _, _}, ResSingle), + {ok, IoList2, Checksum2} = ResSingle, + ?assertEqual(Bin, iolist_to_binary(IoList2)), + ?assertEqual(<<>>, Checksum2). + +% Remove this test when removing append_bin compatibility clause for online +% upgrades from couch_file +% +should_handle_append_bin_upgrade_clause(Fd) -> + Bin = <<"bin">>, + Chunk = couch_file:assemble_file_chunk_and_checksum(Bin), + ResList = gen_server:call(Fd, {append_bins, [Chunk]}, infinity), + ?assertMatch({ok, [_]}, ResList), + {ok, [{Pos1, Len1}]} = ResList, + ?assertEqual({ok, Bin}, couch_file:pread_binary(Fd, Pos1)), + ?assert(is_integer(Len1)), + % size of binary + checksum + ?assert(Len1 > byte_size(Bin) + 16), + ResSingle = gen_server:call(Fd, {append_bin, Chunk}, infinity), + ?assertMatch({ok, _, _}, ResSingle), + {ok, Pos2, Len2} = ResSingle, + ?assert(is_integer(Len2)), + % size of binary + checksum + ?assert(Len2 > byte_size(Bin) + 16), + ?assertEqual(Pos2, Len1), + ?assertEqual({ok, Bin}, couch_file:pread_binary(Fd, Pos2)). + header_test_() -> { "File header read/write tests",
