Patch: Refactor ngx_http_write_request_body into a filter

[grrm grrm]

Hello,

As I said before, "refactoring" change's purpose was to be a first
step to implement steamed request body sending to backend. Looking at
the implementation of the response processing, I saw that all the
events are handled by the upstream module. By that I mean that the
read and write event handlers on the client and upstream socket are
functions in the upstream module. Wouldn't it be appropriate that the
request body reading and sending part be implemented in the upstream
module too?

I say that because the request_body_filter looks more like the
upstream input_filter rather than the ngx_http_output_filter.
Taking also into consideration the patch for inserting custom request
body filters. If this is true, now I understand what you meant by "not
clear it should work this way" :)

Thank you.


2014-07-25 0:00 GMT+03:00 Maxim Dounin <mdounin at mdounin.ru>:
> Hello!
>
> On Thu, Jul 24, 2014 at 11:33:24PM +0300, grrm grrm wrote:
>
>> Hi! Thanks for the response. Quick question, I set the flush flag only
>> when the request body buffer is full. Is it still a problem?
>
> This may be ok if it's then handled and removed, and won't stay in
> the resulting body buffers chain.
>
>>
>> 2014-07-24 23:04 GMT+03:00 Maxim Dounin <mdounin at mdounin.ru>:
>> > Hello!
>> >
>> > On Wed, Jul 23, 2014 at 10:57:41PM +0300, grrm grrm wrote:
>> >
>> >> Hello, Maxim.
>> >> I would like to invest some time into this. Do you point out any
>> >> specific mistakes I made, or where code just doesn't look right? Or it
>> >> is just that a move to the request filters would imply some design
>> >> changes that would have to be considered more carefully?
>> >
>> > In this particular patch you are trying to make a filter from the
>> > ngx_http_write_request_body() function, and you are moving
>> > existing save filter into it - instead of using already existing
>> > save filter, and using the ngx_http_write_request_body() in it.
>> >
>> >> Although I didn't like setting flush in the request body read
>> >> function, I saw that flush is set for all buffers in the proxy input
>> >> filter for example in the non-buffered logic.
>> >
>> > Unconditionally setting the flush flag looks wrong - in
>> > particular, this will cause unneeded flushes when sending large
>> > bodies over SSL connections.
>> >
>> > --
>> > Maxim Dounin
>> > http://nginx.org/
>> >
>> > _______________________________________________
>> > nginx-devel mailing list
>> > nginx-devel at nginx.org
>> > http://mailman.nginx.org/mailman/listinfo/nginx-devel
>>
>> _______________________________________________
>> nginx-devel mailing list
>> nginx-devel at nginx.org
>> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
> --
> Maxim Dounin
> http://nginx.org/
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel