Patch: Refactor ngx_http_write_request_body into a filter

[grrm grrm]

Hi!

Did you have a chance to look at the code?

Any answer is appreciated.

Thank you.

2014-07-02 0:54 GMT+03:00 grrm grrm <grrm77 at gmail.com>:
> 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