[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [users@httpd] mod_ratelimit working by steps ?

2018-06-04 16:22 GMT+02:00 Luca Toscano <toscano.luca@xxxxxxxxx>:
Hi Yann!

2018-06-04 15:59 GMT+02:00 Yann Ylavic <ylavic.dev@xxxxxxxxx>:
Hi Luca,

On Mon, Jun 4, 2018 at 11:23 AM, Luca Toscano <toscano.luca@xxxxxxxxx> wrote:
> To keep archives happy: opened
> https://bz.apache.org/bugzilla/show_bug.cgi?id=62362 and added a patch in
> there, if anybody wants to review it and give me suggestions I'd be happy :)

The semantic of tmpbb is not very clear in your patch, it's both the
brigade where buckets are saved (for the next call?) and the one that
gets passed to the next filter finally.
It doesn't look right to me, if tmpbb is to be forwarded in the same
pass there is no need to change its buckets' lifetime.
Couldn't we ap_save_brigade(f, &ctx->holdingbb, &ctx->tmpbb, ..) at
the end of the filter only?

Thanks for the review, bare in mind that this is my first "big" patch so I'd probably need a better grasp of the internals first :) I haven't touched holdingbb since I saw that was used elsewhere (in RATE_FULLSPEED), but I can try to check it as well. The idea of my patch (that I aimed to) is to pass the ctx->tmbbb only if it reaches chunk_size (or EOS is seen) and buffer otherwise the buckets in it using ap_save_brigade (waiting for the next call to see if chunk_size is reached).

So if I got your point correctly you would use ctx->holdingbb to store the buckets (and changing their lifetime possibly) between calls, and tmpbb only within the same filter invocation?

After re-reading the code and I think I got your point Yann, I'll try to re-work my idea and create a new patch. Thanks!