[
https://issues.apache.org/jira/browse/COUCHDB-864?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900787#action_12900787
]
Adam Kocoloski commented on COUCHDB-864:
----------------------------------------
Repeating a bit more of the IRC discussion, the process dictionary hack is
needed because we spawn another process to actually receive the data off the
socket. mochi sets the mochiweb_request_recv flag in that process, but then
checks for the presence of the flag in the connection-handling process when
deciding whether to close.
I think the main problem with the patch as it exists is the use of
couch_httpd:recv(Req, Remaining). This will try to receive all remaining bytes
off the socket in one go. We should instead use recv(Req, 0), check if we read
too many bytes, and then gen_tcp:unrecv the remainder. The pattern for how to
do this can be found in mochiweb_request:stream_unchunked_body()
Another minor issue is that if the content-length header is missing we
potentially read too many bytes off the socket. In that case the connection
probably should be closed after the request.
> multipart/related PUT's always close the connection.
> ----------------------------------------------------
>
> Key: COUCHDB-864
> URL: https://issues.apache.org/jira/browse/COUCHDB-864
> Project: CouchDB
> Issue Type: Bug
> Components: Database Core
> Reporter: Robert Newson
>
> I noticed that mochiweb always closes the connection when doing a
> multipart/related PUT (to insert the JSON document and accompanying
> attachments in one call). Ultimately it's because we call recv(0) and not
> recv_body, thus consuming more data than we actually process. Mochiweb
> notices that there is unread data on the socket and closes the connection.
> This impacts replication with attachments, as I believe they go through this
> code path (and, thus, are forever reconnecting).
> The code below demonstrates a fix for this issue but isn't good enough for
> trunk. Adam provided the important process dictionary fix.
> ---
> src/couchdb/couch_doc.erl | 1 +
> src/couchdb/couch_httpd_db.erl | 13 +++++++++----
> 2 files changed, 10 insertions(+), 4 deletions(-)
> diff --git a/src/couchdb/couch_doc.erl b/src/couchdb/couch_doc.erl
> index 5009f8f..f8c874b 100644
> --- a/src/couchdb/couch_doc.erl
> +++ b/src/couchdb/couch_doc.erl
> @@ -455,6 +455,7 @@ doc_from_multi_part_stream(ContentType, DataFun) ->
> Parser ! {get_doc_bytes, self()},
> receive
> {doc_bytes, DocBytes} ->
> + erlang:put(mochiweb_request_recv, true),
> Doc = from_json_obj(?JSON_DECODE(DocBytes)),
> % go through the attachments looking for 'follows' in the data,
> % replace with function that reads the data from MIME stream.
> diff --git a/src/couchdb/couch_httpd_db.erl b/src/couchdb/couch_httpd_db.erl
> index b0fbe8d..eff7d67 100644
> --- a/src/couchdb/couch_httpd_db.erl
> +++ b/src/couchdb/couch_httpd_db.erl
> @@ -651,12 +651,13 @@ db_doc_req(#httpd{method='PUT'}=Req, Db, DocId) ->
> } = parse_doc_query(Req),
> couch_doc:validate_docid(DocId),
>
> + Len = couch_httpd:header_value(Req,"Content-Length"),
> Loc = absolute_uri(Req, "/" ++ ?b2l(Db#db.name) ++ "/" ++ ?b2l(DocId)),
> RespHeaders = [{"Location", Loc}],
> case couch_util:to_list(couch_httpd:header_value(Req, "Content-Type")) of
> ("multipart/related;" ++ _) = ContentType ->
> {ok, Doc0} = couch_doc:doc_from_multi_part_stream(ContentType,
> - fun() -> receive_request_data(Req) end),
> + fun() -> receive_request_data(Req, Len) end),
> Doc = couch_doc_from_req(Req, DocId, Doc0),
> update_doc(Req, Db, DocId, Doc, RespHeaders, UpdateType);
> _Else ->
> @@ -775,9 +776,13 @@ send_docs_multipart(Req, Results, Options) ->
> couch_httpd:send_chunk(Resp, <<"--">>),
> couch_httpd:last_chunk(Resp).
>
> -receive_request_data(Req) ->
> - {couch_httpd:recv(Req, 0), fun() -> receive_request_data(Req) end}.
> -
> +receive_request_data(Req, undefined) ->
> + receive_request_data(Req, 0);
> +receive_request_data(Req, Len) when is_list(Len)->
> + Remaining = list_to_integer(Len),
> + Bin = couch_httpd:recv(Req, Remaining),
> + {Bin, fun() -> receive_request_data(Req, Remaining - iolist_size(Bin))
> end}.
> +
> update_doc_result_to_json({{Id, Rev}, Error}) ->
> {_Code, Err, Msg} = couch_httpd:error_info(Error),
> {[{id, Id}, {rev, couch_doc:rev_to_str(Rev)},
> --
> 1.7.2.2
> Umbra
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
