Patch: Refactor ngx_http_write_request_body into a filter

[grrm grrm]

Hello!

What do you think about this patch? It passes the tests and does the
same number of writes to the disk as the old code (at least in the
tests).
>From a readability point of view is also no very bad.

Thanks.

2014-06-27 16:54 GMT+03:00 Maxim Dounin <mdounin at mdounin.ru>:
> Hello!
>
> On Thu, Jun 26, 2014 at 10:07:55PM +0300, grrm grrm wrote:
>
>> 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?
>
> In theory it should be possible to use event pipe in any
> direction.  But I don't think that it would be easy to
> integrate it with various request body requirements.
>
>> 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.
>
> I've previously posted an experimental patch which introduces an
> ability to insert filters into request body chain, which can be
> considered as a "work in this direction".
>
> I think it should be implemented as another filter in
> the request body filter chain.  It will likely require various
> modification to the request body reading code though.
>
> --
> Maxim Dounin
> http://nginx.org/
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: request_body2.patch
Type: application/octet-stream
Size: 11871 bytes
Desc: not available
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20140702/191a3125/attachment.obj>