large attachments and all_docs stopped working since 2.1

[lostnet]

Description

During backups we create a JSON backup of each database using [DB]/_all_docs?include_docs=true&attachments=true so we can search backups and have a binary neutral format.

Steps to Reproduce

• insert a document into a database
• (using fauxton) add 1-2 large attachments to it (at least 2mb starts failures, while by 40mb or so they are pretty certain.) (I used a 6mb png and 30mb pdf in my most recent test.)
• all_docs with include_docs and attachments begins timing out in 5 seconds.
• tweaking all of the new fabric/etc timeout settings and max sizes seems to do nothing.

[notice] 2019-09-26T15:07:49.207007Z couchdb@couchregistrar <0.29645.4> f02116e9b2 couchregistrar:5984 10.1.3.1 adminuser GET /u_82fde7554b/_all_docs?include_docs=true&attachments=true 200 aborted 5096
[error] 2019-09-26T15:07:49.207246Z couchdb@couchregistrar <0.29645.4> f02116e9b2 Response abnormally terminated: timeout

changing the time per item in recv seems to be necessary since a single large attachment can take several seconds:

diff --git a/src/fabric/src/fabric_view_all_docs.erl b/src/fabric/src/fabric_view_all_docs.erl
index 30c8e8d51..198f29541 100644
--- a/src/fabric/src/fabric_view_all_docs.erl
+++ b/src/fabric/src/fabric_view_all_docs.erl
@@ -125,7 +125,7 @@ go(DbName, _Options, Workers, QueryArgs, Callback, Acc0) ->
         update_seq = case UpdateSeq of true -> []; false -> nil end
     },
     case rexi_utils:recv(Workers, #shard.ref, fun handle_message/3,
-        State, infinity, 5000) of
+        State, infinity, 50000) of
     {ok, NewState} ->
         {ok, NewState#collector.user_acc};
     {timeout, NewState} ->  

Expected Behaviour

1. I think it should be possible to tune one timeout in the configuration and have all unspecified timeouts match the timeout rather than needing to tune each newly invented timeout to prevent existing systems from failing on update based on rather random specifics of our data and infrastructure.
2. The log should report which timeout was hit. There are now more than a dozen possible things "timeout" could mean that can be configured and a few that can not.

Your Environment

• CouchDB Version used: 2.3.1-17f8c2265
• Browser name and version: chrome 75
• Operating System and version: debian 9 (fix tested on amd64 node, bug on both amd65 and arm7 nodes.)

Additional context

Fix badmatch in fabric_view_all_docs #2153 looks similar, but seems to be for the call with key arguments.

[lostnet]

Due to find not supporting attachments I don't see a simple alternative to fixing all_docs to not have a hardcoded per record timeout. I have put together an example of making it conditional on whether attachments are in use, but never hardcoded as an example of the pattern I think is necessary to not cause timeouts that can't be worked around on older hardware, larger docs and attachments, etc:

diff --git a/src/fabric/src/fabric_view_all_docs.erl b/src/fabric/src/fabric_view_all_docs.erl
index 07cd1b180..6319f5bd2 100644
--- a/src/fabric/src/fabric_view_all_docs.erl
+++ b/src/fabric/src/fabric_view_all_docs.erl
@@ -119,8 +119,14 @@ go(DbName, Options, QueryArgs, Callback, Acc0) ->
         Callback({error, timeout}, Acc0)
     end.
 
+msg_timeout(DocOpts) ->
+	case lists:member(attachments, DocOpts) of
+		false -> fabric_util:request_timeout();
+		_ -> fabric_util:attachments_timeout()
+	end.
+
 go(DbName, _Options, Workers, QueryArgs, Callback, Acc0) ->
-    #mrargs{limit = Limit, skip = Skip, update_seq = UpdateSeq} = QueryArgs,
+    #mrargs{limit = Limit, skip = Skip, update_seq = UpdateSeq, doc_options = DocOpts} = QueryArgs,
     State = #collector{
         db_name = DbName,
         query_args = QueryArgs,
@@ -132,7 +138,7 @@ go(DbName, _Options, Workers, QueryArgs, Callback, Acc0) ->
         update_seq = case UpdateSeq of true -> []; false -> nil end
     },
     case rexi_utils:recv(Workers, #shard.ref, fun handle_message/3,
-        State, fabric_util:view_timeout(QueryArgs), 5000) of
+        State, fabric_util:view_timeout(QueryArgs), msg_timeout(DocOpts)) of
     {ok, NewState} ->
         {ok, NewState#collector.user_acc};
     {timeout, NewState} ->

[wohali]

@lostnet If you'd like this issue to get more attention, please put up a PR. Thanks.

[lostnet]

@wohali While raising the PerMsgTO above 5000 seems to work around the problem for me, it seems like it is intentionally supposed to do this to detect a failure within 5 seconds? So I don't think I was on the right track..

Looking at the handling of rexi_utils:process_message I followed '$rexi_ping' back to mango avoiding timeouts when using the all_docs index with attachments include_docs (or some other potentials for delay?). I think I should try to make a PR for all_docs to use the same model of pinging before PerMsgTO?

Thanks

[wohali]

@lostnet I'm not the best person to ask on this question, just the issue triager, but if you want to post the specific diff you have above as a PR we can get someone to look at it.

I'll try pinging @davisp or @nickva to see if they can comment prior to that.

[davisp]

That timeout you were originally dealing with probably isn't the right approach. That's a per-message timeout which we'd never expect to have a delay of more than 5s (or else something is quite wrong).

My first guess is that the reading those attachments into RAM to send out in the response is taking too long and triggering the timeout. I'd look to see if the time we're calling a 5s timeout is accidentally including time spent in the callback to rexi_utils:recv call.