Patch: Refactor ngx_http_write_request_body into a filter

[grrm grrm]

Hi!

I managed to fix the write to disk issue, but as you said the code now
looks quite convoluted. Those ifs are horrible. Understandably I
guess, when you try to move logic from different places into one place
but it still depends on external context (rb->buf). My patch was
mostly a try to pave the way to non-buffered request body processing
in way similar to the response processing pipeline where all the work
is done by the filters.

I saw this feature in the tengine fork of nginx, however there the
work is still done by a handler similar to write_to_file. Also, all
the body data need to be copied inside the memory at least one time,
which is not good.

I also looked at the repose pipeline and there are two main methods of
reading from the client - the nonbuffered and ngx_event_pipe_t. Do you
think the pipe could be used in reverse (client->upstream)? Or would
it even make sense to do it that way?

Also, do you have any work done into this direction (if you can
comment on that)? Granted, my attempt was maybe too big a step.

Thank you.

2014-06-20 21:09 GMT+03:00 Maxim Dounin <mdounin at mdounin.ru>:
> Hello!
>
> On Fri, Jun 20, 2014 at 12:56:33AM +0300, grrm grrm wrote:
>
>> Hello,
>>
>> This patch removes some redundant ngx_http_request_body_filter calls,
>> simplifies the ngx_http_do_read_client_request_body and
>> ngx_http_read_client_request_body functions and removes some
>> duplication of code.
>> body.t and body_chunked.t test in nginx-tests are passing.
>>
>> Please kindly consider it.
>
> It looks like the patch introduces at least one serious enough
> problem: with the patch, if disk buffering is used, all reads
> from a client are mapped into disk writes, which is bad.  Also, I
> don't think that it improves things from readability point of
> view.
>
> --
> Maxim Dounin
> http://nginx.org/
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel