You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by Damien Katz <da...@apache.org> on 2009/05/17 23:10:38 UTC
Re: svn commit: r775634 - /couchdb/branches/0.9.x/src/couchdb/couch_db.erl
+%% on rare occasions ibrowse seems to process a chunked response
incorrectly
+%% and include an extra "\r" in the last chunk. This code ensures
that we
+%% truncate the downloaed attachment at the length specified in the
metadata.
That really sucks. We need to either fix ibrowse or drop it altogether.
-Damien
On May 17, 2009, at 8:29 AM, kocolosk@apache.org wrote:
> Author: kocolosk
> Date: Sun May 17 12:29:22 2009
> New Revision: 775634
>
> URL: http://svn.apache.org/viewvc?rev=775634&view=rev
> Log:
> work around ibrowse giving response chunks too many bytes. Thanks
> Antony.
>
> Modified:
> couchdb/branches/0.9.x/src/couchdb/couch_db.erl
>
> Modified: couchdb/branches/0.9.x/src/couchdb/couch_db.erl
> URL: http://svn.apache.org/viewvc/couchdb/branches/0.9.x/src/couchdb/couch_db.erl?rev=775634&r1=775633&r2=775634&view=diff
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- couchdb/branches/0.9.x/src/couchdb/couch_db.erl (original)
> +++ couchdb/branches/0.9.x/src/couchdb/couch_db.erl Sun May 17
> 12:29:22 2009
> @@ -649,12 +649,24 @@
> {ok, SpAcc};
> write_streamed_attachment(Stream, F, LenLeft, nil) ->
> Bin = F(),
> - {ok, StreamPointer} = couch_stream:write(Stream, Bin),
> - write_streamed_attachment(Stream, F, LenLeft - size(Bin),
> StreamPointer);
> + TruncatedBin = check_bin_length(LenLeft, Bin),
> + {ok, SpAcc} = couch_stream:write(Stream, TruncatedBin),
> + write_streamed_attachment(Stream, F, LenLeft -
> size(TruncatedBin), SpAcc);
> write_streamed_attachment(Stream, F, LenLeft, SpAcc) ->
> Bin = F(),
> - {ok, _} = couch_stream:write(Stream, Bin),
> - write_streamed_attachment(Stream, F, LenLeft - size(Bin), SpAcc).
> + TruncatedBin = check_bin_length(LenLeft, Bin),
> + {ok, _} = couch_stream:write(Stream, TruncatedBin),
> + write_streamed_attachment(Stream, F, LenLeft -
> size(TruncatedBin), SpAcc).
> +
> +%% on rare occasions ibrowse seems to process a chunked response
> incorrectly
> +%% and include an extra "\r" in the last chunk. This code ensures
> that we
> +%% truncate the downloaed attachment at the length specified in the
> metadata.
> +check_bin_length(LenLeft, Bin) when size(Bin) > LenLeft ->
> + <<ValidData:LenLeft/binary, Crap/binary>> = Bin,
> + ?LOG_ERROR("write_streamed_attachment has written too much
> expected: ~p" ++
> + " got: ~p tail: ~p", [LenLeft, size(Bin), Crap]),
> + ValidData;
> +check_bin_length(_, Bin) -> Bin.
>
> enum_docs_since_reduce_to_count(Reds) ->
> couch_btree:final_reduce(
>
>
Re: svn commit: r775634 - /couchdb/branches/0.9.x/src/couchdb/couch_db.erl
Posted by Adam Kocoloski <ko...@apache.org>.
On May 19, 2009, at 3:06 AM, Kevin Jackson wrote:
> Hi,
>
>> +%% on rare occasions ibrowse seems to process a chunked response
>> incorrectly
>> +%% and include an extra "\r" in the last chunk. This code ensures
>> that we
>> +%% truncate the downloaed attachment at the length specified in the
>> metadata.
>
> What is the condition that causes ibrowse to add an additional \r?
If we knew, we'd probably have a patch :-/
I sent a test script that does a pretty good job of reproducing the
problem to Chandru yesterday. He said he'd look into it ASAP.
>>
>> That really sucks. We need to either fix ibrowse or drop it
>> altogether.
>
> What other options are there for an erlang http client? I'm presuming
> that the http client included with the erlang inets module was ruled
> out previously?
I'm only aware of inets and ibrowse. We used to use the inets
client. To be honest, it wasn't really that bad. Jason Davies led
the switch to ibrowse with a patch back in January which made
replication more reliable for him. Since then there's been a lot of
work on improving the replicator's reliability in the face of
unreliable links. It could be that inets would work just fine for us
-- I certainly wouldn't say that its been ruled out.
Best, Adam
Re: svn commit: r775634 - /couchdb/branches/0.9.x/src/couchdb/couch_db.erl
Posted by Kevin Jackson <fo...@gmail.com>.
Hi,
> +%% on rare occasions ibrowse seems to process a chunked response
> incorrectly
> +%% and include an extra "\r" in the last chunk. This code ensures that we
> +%% truncate the downloaed attachment at the length specified in the
> metadata.
What is the condition that causes ibrowse to add an additional \r?
>
> That really sucks. We need to either fix ibrowse or drop it altogether.
What other options are there for an erlang http client? I'm presuming
that the http client included with the erlang inets module was ruled
out previously?
Kev
Re: svn commit: r775634 - /couchdb/branches/0.9.x/src/couchdb/couch_db.erl
Posted by Antony Blakey <an...@gmail.com>.
On 18/05/2009, at 8:53 AM, Antony Blakey wrote:
> I'll ask. Apart from being 3.5G it also contains mainly non-public
> work-in-progress client data.
And I haven't been able to isolate it to less data. It may also be
dependent on the fact that I'm pull replicating to an Ubuntu 8.04
server running in VMWare Fusion from the OSX hosting VMWare, using an
SSH tunnel between the two.
Antony Blakey
--------------------------
CTO, Linkuistics Pty Ltd
Ph: 0438 840 787
Human beings, who are almost unique in having the ability to learn
from the experience of others, are also remarkable for their apparent
disinclination to do so.
-- Douglas Adams
Re: svn commit: r775634 - /couchdb/branches/0.9.x/src/couchdb/couch_db.erl
Posted by Antony Blakey <an...@gmail.com>.
On 18/05/2009, at 10:54 AM, Adam Kocoloski wrote:
> Ok, thanks. I've managed to generate a reduced test case with a
> 13KB attachment where a standalone ibrowse request adds a trailing
> "\r" ~20% of the time. I checked with httplib2 that the problem is
> not in CouchDB.
I've just seen this error with a trailing "\n". Same data as before,
but this time replicating from my previously replicated-to ubuntu to a
different ubuntu.
Antony Blakey
-------------
CTO, Linkuistics Pty Ltd
Ph: 0438 840 787
The fact that an opinion has been widely held is no evidence whatever
that it is not utterly absurd.
-- Bertrand Russell
Re: svn commit: r775634 - /couchdb/branches/0.9.x/src/couchdb/couch_db.erl
Posted by Adam Kocoloski <ko...@apache.org>.
Ok, thanks. I've managed to generate a reduced test case with a 13KB
attachment where a standalone ibrowse request adds a trailing "\r"
~20% of the time. I checked with httplib2 that the problem is not in
CouchDB.
I'll clean up the test case and contact Chandru tomorrow.
Adam
On May 17, 2009, at 10:02 PM, Antony Blakey wrote:
>
> On 18/05/2009, at 8:58 AM, Adam Kocoloski wrote:
>
>> Thanks for the tip about the OTP release version, too.
>
> Problem still exists with R13B.
>
> Antony Blakey
> -------------
> CTO, Linkuistics Pty Ltd
> Ph: 0438 840 787
>
> He who would make his own liberty secure, must guard even his enemy
> from repression.
> -- Thomas Paine
>
>
Re: svn commit: r775634 - /couchdb/branches/0.9.x/src/couchdb/couch_db.erl
Posted by Antony Blakey <an...@gmail.com>.
On 18/05/2009, at 8:58 AM, Adam Kocoloski wrote:
> Thanks for the tip about the OTP release version, too.
Problem still exists with R13B.
Antony Blakey
-------------
CTO, Linkuistics Pty Ltd
Ph: 0438 840 787
He who would make his own liberty secure, must guard even his enemy
from repression.
-- Thomas Paine
Re: svn commit: r775634 - /couchdb/branches/0.9.x/src/couchdb/couch_db.erl
Posted by Adam Kocoloski <ko...@apache.org>.
Commercial-in-confidence works for me. Thanks for the tip about the
OTP release version, too. Best,
Adam
On May 17, 2009, at 8:53 PM, Antony Blakey wrote:
>
> On 18/05/2009, at 8:44 AM, Adam Kocoloski wrote:
>
>> Sure, I agree. Consider this commit a stopgap until we're able to
>> pin down the source of the problem. Antony, any chance you can
>> make that 100% reproducible case publicly available?
>
> I'll ask. Apart from being 3.5G it also contains mainly non-public
> work-in-progress client data.
>
> I may be able to make it available selectively with a commercial-in-
> confidence rider. Would that help? My replication is still
> unreliable so I need to track that down, and hence don't have time
> to fix the cause of this bug given that I have this workaround.
>
> BTW: I've just moved to R13B, and I think the problem isn't
> occurring - I'm chasing it down. If that's the case then we can
> chalk it up to an erlang bug.
>
>>
>> Adam
>>
>> On May 17, 2009, at 5:10 PM, Damien Katz wrote:
>>
>>> +%% on rare occasions ibrowse seems to process a chunked response
>>> incorrectly
>>> +%% and include an extra "\r" in the last chunk. This code
>>> ensures that we
>>> +%% truncate the downloaed attachment at the length specified in
>>> the metadata.
>>>
>>> That really sucks. We need to either fix ibrowse or drop it
>>> altogether.
>>>
>>> -Damien
>>>
>>> On May 17, 2009, at 8:29 AM, kocolosk@apache.org wrote:
>>>
>>>> Author: kocolosk
>>>> Date: Sun May 17 12:29:22 2009
>>>> New Revision: 775634
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=775634&view=rev
>>>> Log:
>>>> work around ibrowse giving response chunks too many bytes.
>>>> Thanks Antony.
>>>>
>>>> Modified:
>>>> couchdb/branches/0.9.x/src/couchdb/couch_db.erl
>>>>
>>>> Modified: couchdb/branches/0.9.x/src/couchdb/couch_db.erl
>>>> URL: http://svn.apache.org/viewvc/couchdb/branches/0.9.x/src/couchdb/couch_db.erl?rev=775634&r1=775633&r2=775634&view=diff
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> =
>>>> ===================================================================
>>>> --- couchdb/branches/0.9.x/src/couchdb/couch_db.erl (original)
>>>> +++ couchdb/branches/0.9.x/src/couchdb/couch_db.erl Sun May 17
>>>> 12:29:22 2009
>>>> @@ -649,12 +649,24 @@
>>>> {ok, SpAcc};
>>>> write_streamed_attachment(Stream, F, LenLeft, nil) ->
>>>> Bin = F(),
>>>> - {ok, StreamPointer} = couch_stream:write(Stream, Bin),
>>>> - write_streamed_attachment(Stream, F, LenLeft - size(Bin),
>>>> StreamPointer);
>>>> + TruncatedBin = check_bin_length(LenLeft, Bin),
>>>> + {ok, SpAcc} = couch_stream:write(Stream, TruncatedBin),
>>>> + write_streamed_attachment(Stream, F, LenLeft -
>>>> size(TruncatedBin), SpAcc);
>>>> write_streamed_attachment(Stream, F, LenLeft, SpAcc) ->
>>>> Bin = F(),
>>>> - {ok, _} = couch_stream:write(Stream, Bin),
>>>> - write_streamed_attachment(Stream, F, LenLeft - size(Bin),
>>>> SpAcc).
>>>> + TruncatedBin = check_bin_length(LenLeft, Bin),
>>>> + {ok, _} = couch_stream:write(Stream, TruncatedBin),
>>>> + write_streamed_attachment(Stream, F, LenLeft -
>>>> size(TruncatedBin), SpAcc).
>>>> +
>>>> +%% on rare occasions ibrowse seems to process a chunked response
>>>> incorrectly
>>>> +%% and include an extra "\r" in the last chunk. This code
>>>> ensures that we
>>>> +%% truncate the downloaed attachment at the length specified in
>>>> the metadata.
>>>> +check_bin_length(LenLeft, Bin) when size(Bin) > LenLeft ->
>>>> + <<ValidData:LenLeft/binary, Crap/binary>> = Bin,
>>>> + ?LOG_ERROR("write_streamed_attachment has written too much
>>>> expected: ~p" ++
>>>> + " got: ~p tail: ~p", [LenLeft, size(Bin), Crap]),
>>>> + ValidData;
>>>> +check_bin_length(_, Bin) -> Bin.
>>>>
>>>> enum_docs_since_reduce_to_count(Reds) ->
>>>> couch_btree:final_reduce(
>>>>
>>>>
>>>
>>
>
> Antony Blakey
> --------------------------
> CTO, Linkuistics Pty Ltd
> Ph: 0438 840 787
>
> The project was so plagued by politics and ego that when the
> engineers requested technical oversight, our manager hired a
> psychologist instead.
> -- Ron Avitzur
>
Re: svn commit: r775634 - /couchdb/branches/0.9.x/src/couchdb/couch_db.erl
Posted by Antony Blakey <an...@gmail.com>.
On 18/05/2009, at 8:44 AM, Adam Kocoloski wrote:
> Sure, I agree. Consider this commit a stopgap until we're able to
> pin down the source of the problem. Antony, any chance you can make
> that 100% reproducible case publicly available?
I'll ask. Apart from being 3.5G it also contains mainly non-public
work-in-progress client data.
I may be able to make it available selectively with a commercial-in-
confidence rider. Would that help? My replication is still unreliable
so I need to track that down, and hence don't have time to fix the
cause of this bug given that I have this workaround.
BTW: I've just moved to R13B, and I think the problem isn't occurring
- I'm chasing it down. If that's the case then we can chalk it up to
an erlang bug.
>
> Adam
>
> On May 17, 2009, at 5:10 PM, Damien Katz wrote:
>
>> +%% on rare occasions ibrowse seems to process a chunked response
>> incorrectly
>> +%% and include an extra "\r" in the last chunk. This code ensures
>> that we
>> +%% truncate the downloaed attachment at the length specified in
>> the metadata.
>>
>> That really sucks. We need to either fix ibrowse or drop it
>> altogether.
>>
>> -Damien
>>
>> On May 17, 2009, at 8:29 AM, kocolosk@apache.org wrote:
>>
>>> Author: kocolosk
>>> Date: Sun May 17 12:29:22 2009
>>> New Revision: 775634
>>>
>>> URL: http://svn.apache.org/viewvc?rev=775634&view=rev
>>> Log:
>>> work around ibrowse giving response chunks too many bytes. Thanks
>>> Antony.
>>>
>>> Modified:
>>> couchdb/branches/0.9.x/src/couchdb/couch_db.erl
>>>
>>> Modified: couchdb/branches/0.9.x/src/couchdb/couch_db.erl
>>> URL: http://svn.apache.org/viewvc/couchdb/branches/0.9.x/src/couchdb/couch_db.erl?rev=775634&r1=775633&r2=775634&view=diff
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> =
>>> ====================================================================
>>> --- couchdb/branches/0.9.x/src/couchdb/couch_db.erl (original)
>>> +++ couchdb/branches/0.9.x/src/couchdb/couch_db.erl Sun May 17
>>> 12:29:22 2009
>>> @@ -649,12 +649,24 @@
>>> {ok, SpAcc};
>>> write_streamed_attachment(Stream, F, LenLeft, nil) ->
>>> Bin = F(),
>>> - {ok, StreamPointer} = couch_stream:write(Stream, Bin),
>>> - write_streamed_attachment(Stream, F, LenLeft - size(Bin),
>>> StreamPointer);
>>> + TruncatedBin = check_bin_length(LenLeft, Bin),
>>> + {ok, SpAcc} = couch_stream:write(Stream, TruncatedBin),
>>> + write_streamed_attachment(Stream, F, LenLeft -
>>> size(TruncatedBin), SpAcc);
>>> write_streamed_attachment(Stream, F, LenLeft, SpAcc) ->
>>> Bin = F(),
>>> - {ok, _} = couch_stream:write(Stream, Bin),
>>> - write_streamed_attachment(Stream, F, LenLeft - size(Bin),
>>> SpAcc).
>>> + TruncatedBin = check_bin_length(LenLeft, Bin),
>>> + {ok, _} = couch_stream:write(Stream, TruncatedBin),
>>> + write_streamed_attachment(Stream, F, LenLeft -
>>> size(TruncatedBin), SpAcc).
>>> +
>>> +%% on rare occasions ibrowse seems to process a chunked response
>>> incorrectly
>>> +%% and include an extra "\r" in the last chunk. This code
>>> ensures that we
>>> +%% truncate the downloaed attachment at the length specified in
>>> the metadata.
>>> +check_bin_length(LenLeft, Bin) when size(Bin) > LenLeft ->
>>> + <<ValidData:LenLeft/binary, Crap/binary>> = Bin,
>>> + ?LOG_ERROR("write_streamed_attachment has written too much
>>> expected: ~p" ++
>>> + " got: ~p tail: ~p", [LenLeft, size(Bin), Crap]),
>>> + ValidData;
>>> +check_bin_length(_, Bin) -> Bin.
>>>
>>> enum_docs_since_reduce_to_count(Reds) ->
>>> couch_btree:final_reduce(
>>>
>>>
>>
>
Antony Blakey
--------------------------
CTO, Linkuistics Pty Ltd
Ph: 0438 840 787
The project was so plagued by politics and ego that when the engineers
requested technical oversight, our manager hired a psychologist instead.
-- Ron Avitzur
Re: svn commit: r775634 - /couchdb/branches/0.9.x/src/couchdb/couch_db.erl
Posted by Adam Kocoloski <ko...@apache.org>.
Sure, I agree. Consider this commit a stopgap until we're able to pin
down the source of the problem. Antony, any chance you can make that
100% reproducible case publicly available?
Adam
On May 17, 2009, at 5:10 PM, Damien Katz wrote:
> +%% on rare occasions ibrowse seems to process a chunked response
> incorrectly
> +%% and include an extra "\r" in the last chunk. This code ensures
> that we
> +%% truncate the downloaed attachment at the length specified in the
> metadata.
>
> That really sucks. We need to either fix ibrowse or drop it
> altogether.
>
> -Damien
>
> On May 17, 2009, at 8:29 AM, kocolosk@apache.org wrote:
>
>> Author: kocolosk
>> Date: Sun May 17 12:29:22 2009
>> New Revision: 775634
>>
>> URL: http://svn.apache.org/viewvc?rev=775634&view=rev
>> Log:
>> work around ibrowse giving response chunks too many bytes. Thanks
>> Antony.
>>
>> Modified:
>> couchdb/branches/0.9.x/src/couchdb/couch_db.erl
>>
>> Modified: couchdb/branches/0.9.x/src/couchdb/couch_db.erl
>> URL: http://svn.apache.org/viewvc/couchdb/branches/0.9.x/src/couchdb/couch_db.erl?rev=775634&r1=775633&r2=775634&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- couchdb/branches/0.9.x/src/couchdb/couch_db.erl (original)
>> +++ couchdb/branches/0.9.x/src/couchdb/couch_db.erl Sun May 17
>> 12:29:22 2009
>> @@ -649,12 +649,24 @@
>> {ok, SpAcc};
>> write_streamed_attachment(Stream, F, LenLeft, nil) ->
>> Bin = F(),
>> - {ok, StreamPointer} = couch_stream:write(Stream, Bin),
>> - write_streamed_attachment(Stream, F, LenLeft - size(Bin),
>> StreamPointer);
>> + TruncatedBin = check_bin_length(LenLeft, Bin),
>> + {ok, SpAcc} = couch_stream:write(Stream, TruncatedBin),
>> + write_streamed_attachment(Stream, F, LenLeft -
>> size(TruncatedBin), SpAcc);
>> write_streamed_attachment(Stream, F, LenLeft, SpAcc) ->
>> Bin = F(),
>> - {ok, _} = couch_stream:write(Stream, Bin),
>> - write_streamed_attachment(Stream, F, LenLeft - size(Bin),
>> SpAcc).
>> + TruncatedBin = check_bin_length(LenLeft, Bin),
>> + {ok, _} = couch_stream:write(Stream, TruncatedBin),
>> + write_streamed_attachment(Stream, F, LenLeft -
>> size(TruncatedBin), SpAcc).
>> +
>> +%% on rare occasions ibrowse seems to process a chunked response
>> incorrectly
>> +%% and include an extra "\r" in the last chunk. This code ensures
>> that we
>> +%% truncate the downloaed attachment at the length specified in
>> the metadata.
>> +check_bin_length(LenLeft, Bin) when size(Bin) > LenLeft ->
>> + <<ValidData:LenLeft/binary, Crap/binary>> = Bin,
>> + ?LOG_ERROR("write_streamed_attachment has written too much
>> expected: ~p" ++
>> + " got: ~p tail: ~p", [LenLeft, size(Bin), Crap]),
>> + ValidData;
>> +check_bin_length(_, Bin) -> Bin.
>>
>> enum_docs_since_reduce_to_count(Reds) ->
>> couch_btree:final_reduce(
>>
>>
>