You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by ja...@apache.org on 2021/08/26 14:48:03 UTC

[couchdb] 01/01: fix: avoid dropping attachment chunks on quorum writes

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

jan pushed a commit to branch fix/attachment-writers-larger-n
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 8a4d45bd198ee59af3300dfb953cea8723d56432
Author: Jan Lehnardt <ja...@apache.org>
AuthorDate: Thu Aug 26 16:41:58 2021 +0200

    fix: avoid dropping attachment chunks on quorum writes
    
    This only applies to databases that have an n > [cluster] n.
    
    Our `middleman()` function that proxies attachment streams from
    the incoming HTTP socket on the coordinating node to the target
    shard-bearing nodes used the server config to determine whether
    it should start dropping chunks from the stream.
    
    If a database was created with a larger `n`, the `middleman()`
    function could have started to drop attachment chunks before
    all attached nodes had a chance to receive it.
    
    This fix uses a database’s concrete `n` value rather than the
    server config default value.
    
    Co-Authored-By: James Coglan <jc...@gmail.com>
    Co-AUthored-By: Robert Newson <rn...@apache.org>
---
 src/chttpd/src/chttpd_db.erl       |  2 +-
 src/fabric/src/fabric.erl          |  8 ++++----
 src/fabric/src/fabric_doc_atts.erl | 26 +++++++++++++-------------
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl
index ffdab2a..31ada8f 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -1571,7 +1571,7 @@ db_attachment_req(#httpd{method=Method, user_ctx=Ctx}=Req, Db, DocId, FileNamePa
                 undefined -> <<"application/octet-stream">>;
                 CType -> list_to_binary(CType)
             end,
-            Data = fabric:att_receiver(Req, chttpd:body_length(Req)),
+            Data = fabric:att_receiver(Req, couch_db:name(Db), chttpd:body_length(Req)),
             ContentLen = case couch_httpd:header_value(Req,"Content-Length") of
                 undefined -> undefined;
                 Length -> list_to_integer(Length)
diff --git a/src/fabric/src/fabric.erl b/src/fabric/src/fabric.erl
index 27fa8c0..34b967d 100644
--- a/src/fabric/src/fabric.erl
+++ b/src/fabric/src/fabric.erl
@@ -28,7 +28,7 @@
 % Documents
 -export([open_doc/3, open_revs/4, get_doc_info/3, get_full_doc_info/3,
     get_missing_revs/2, get_missing_revs/3, update_doc/3, update_docs/3,
-    purge_docs/3, att_receiver/2]).
+    purge_docs/3, att_receiver/3]).
 
 % Views
 -export([all_docs/4, all_docs/5, changes/4, query_view/3, query_view/4,
@@ -324,11 +324,11 @@ purge_docs(DbName, IdsRevs, Options) when is_list(IdsRevs) ->
 %%      returns a fabric attachment receiver context tuple
 %%      with the spawned middleman process, an empty binary,
 %%      or exits with an error tuple {Error, Arg}
--spec att_receiver(#httpd{}, Length :: undefined | chunked | pos_integer() |
+-spec att_receiver(#httpd{}, dbname(), Length :: undefined | chunked | pos_integer() |
         {unknown_transfer_encoding, any()}) ->
     {fabric_attachment_receiver, pid(), chunked | pos_integer()} | binary().
-att_receiver(Req, Length) ->
-    fabric_doc_atts:receiver(Req, Length).
+att_receiver(Req, DbName, Length) ->
+    fabric_doc_atts:receiver(Req, DbName, Length).
 
 %% @equiv all_docs(DbName, [], Callback, Acc0, QueryArgs)
 all_docs(DbName, Callback, Acc, QueryArgs) ->
diff --git a/src/fabric/src/fabric_doc_atts.erl b/src/fabric/src/fabric_doc_atts.erl
index 65ba65f..bd0687b 100644
--- a/src/fabric/src/fabric_doc_atts.erl
+++ b/src/fabric/src/fabric_doc_atts.erl
@@ -18,25 +18,25 @@
 -include_lib("couch/include/couch_db.hrl").
 
 -export([
-    receiver/2,
+    receiver/3,
     receiver_callback/2
 ]).
 
 
-receiver(_Req, undefined) ->
+receiver(_Req, _DbName, undefined) ->
     <<"">>;
-receiver(_Req, {unknown_transfer_encoding, Unknown}) ->
+receiver(_Req, _DbName, {unknown_transfer_encoding, Unknown}) ->
     exit({unknown_transfer_encoding, Unknown});
-receiver(Req, chunked) ->
-    MiddleMan = spawn(fun() -> middleman(Req, chunked) end),
+receiver(Req, DbName, chunked) ->
+    MiddleMan = spawn(fun() -> middleman(Req, DbName, chunked) end),
     {fabric_attachment_receiver, MiddleMan, chunked};
-receiver(_Req, 0) ->
+receiver(_Req, _DbName, 0) ->
     <<"">>;
-receiver(Req, Length) when is_integer(Length) ->
+receiver(Req, DbName, Length) when is_integer(Length) ->
     maybe_send_continue(Req),
-    Middleman = spawn(fun() -> middleman(Req, Length) end),
+    Middleman = spawn(fun() -> middleman(Req, DbName, Length) end),
     {fabric_attachment_receiver, Middleman, Length};
-receiver(_Req, Length) ->
+receiver(_Req, _DbName, Length) ->
     exit({length_not_integer, Length}).
 
 
@@ -108,7 +108,7 @@ receive_unchunked_attachment(Req, Length) ->
     end,
     receive_unchunked_attachment(Req, Length - size(Data)).
 
-middleman(Req, chunked) ->
+middleman(Req, DbName, chunked) ->
     % spawn a process to actually receive the uploaded data
     RcvFun = fun(ChunkRecord, ok) ->
         receive {From, go} -> From ! {self(), ChunkRecord} end, ok
@@ -116,13 +116,13 @@ middleman(Req, chunked) ->
     Receiver = spawn(fun() -> couch_httpd:recv_chunked(Req,4096,RcvFun,ok) end),
 
     % take requests from the DB writers and get data from the receiver
-    N = config:get_integer("cluster", "n", 3),
+    N = mem3:n(DbName),
     Timeout = fabric_util:attachments_timeout(),
     middleman_loop(Receiver, N, [], [], Timeout);
 
-middleman(Req, Length) ->
+middleman(Req, DbName, Length) ->
     Receiver = spawn(fun() -> receive_unchunked_attachment(Req, Length) end),
-    N = config:get_integer("cluster", "n", 3),
+    N = mem3:n(DbName),
     Timeout = fabric_util:attachments_timeout(),
     middleman_loop(Receiver, N, [], [], Timeout).