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 2025/03/05 06:06:03 UTC
(couchdb) branch main updated: Fix compatibility clause for attachments
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/couchdb.git
The following commit(s) were added to refs/heads/main by this push:
new 1a88c2a76 Fix compatibility clause for attachments
1a88c2a76 is described below
commit 1a88c2a762ec7f62f3b4a056fbfe8d87022a77f0
Author: Nick Vatamaniuc <va...@gmail.com>
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",