> Am 14.10.2021 um 11:28 schrieb Ruediger Pluem <rpl...@apache.org>:
> 
> 
> 
> On 10/14/21 11:20 AM, ste...@eissing.org wrote:
>> 
>> 
>>> Am 14.10.2021 um 11:17 schrieb Ruediger Pluem <rpl...@apache.org>:
>>> 
>>> 
>>> 
>>> On 10/14/21 10:59 AM, ic...@apache.org wrote:
>>>> Author: icing
>>>> Date: Thu Oct 14 08:59:12 2021
>>>> New Revision: 1894220
>>>> 
>>>> URL: http://svn.apache.org/viewvc?rev=1894220&view=rev
>>>> Log:
>>>> *) mod_http2: no longer splitting buckets on adding them to a beam,
>>>>    accepting the whole bucket since no memory is saved by a split.
>>>>    Also, allowing meta buckets to be added to a "full" beam.
>>>>    Re-enabled test cases for travis verification.
>>>> 
>>>> 
>>>> Modified:
>>>>   httpd/httpd/trunk/modules/http2/h2_bucket_beam.c
>>>>   httpd/httpd/trunk/test/modules/http2/test_004_post.py
>>>>   httpd/httpd/trunk/test/modules/http2/test_400_push.py
>>>>   httpd/httpd/trunk/test/modules/http2/test_401_early_hints.py
>>>> 
>>>> Modified: httpd/httpd/trunk/modules/http2/h2_bucket_beam.c
>>>> URL: 
>>>> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/http2/h2_bucket_beam.c?rev=1894220&r1=1894219&r2=1894220&view=diff
>>>> ==============================================================================
>>>> --- httpd/httpd/trunk/modules/http2/h2_bucket_beam.c (original)
>>>> +++ httpd/httpd/trunk/modules/http2/h2_bucket_beam.c Thu Oct 14 08:59:12 
>>>> 2021
>>>> @@ -420,29 +420,41 @@ void h2_beam_abort(h2_bucket_beam *beam,
>>>> }
>>>> 
>>>> static apr_status_t append_bucket(h2_bucket_beam *beam,
>>>> -                                  apr_bucket *b,
>>>> +                                  apr_bucket_brigade *bb,
>>>>                                  apr_read_type_e block,
>>>>                                  apr_size_t *pspace_left,
>>>>                                  apr_off_t *pwritten)
>>>> {
>>>> +    apr_bucket *b;
>>>>    const char *data;
>>>>    apr_size_t len;
>>>> -    apr_status_t status = APR_SUCCESS;
>>>> -    int can_beam = 0, check_len;
>>>> +    apr_status_t rv = APR_SUCCESS;
>>>> +    int can_beam = 0;
>>>> 
>>>>    (void)block;
>>>>    if (beam->aborted) {
>>>> -        return APR_ECONNABORTED;
>>>> +        rv = APR_ECONNABORTED;
>>>> +        goto cleanup;
>>>>    }
>>>> -    
>>>> +
>>>> +    b = APR_BRIGADE_FIRST(bb);
>>>>    if (APR_BUCKET_IS_METADATA(b)) {
>>>>        APR_BUCKET_REMOVE(b);
>>>>        apr_bucket_setaside(b, beam->pool);
>>>>        H2_BLIST_INSERT_TAIL(&beam->buckets_to_send, b);
>>>>        *pwritten += (apr_off_t)b->length;
>>> 
>>> Are there meta buckets that have a length that is not zero?
>>> I mean is it even allowed to define meta buckets with a length != 0?
>> 
>> We need the bucket police for that one!
>> 
>> There is currently the H2HEADER bucket that does this shameful thing. It 
>> should probably not do that and find another way.
> 
> I am personally fine with metabuckets that have a non zero length, but I fear 
> there could be assumptions in the existing code
> that metabucket means length == 0. If this assumption is only there for 
> memory accounting I guess it does not matter as
> metabuckets with length != 0 likely only have a small length.
> Some I am keen to hear what others think about non zero length metabuckets.

Background: this "hack" exists, because mod_logio reports overall data and 
response body data separately. And this counts (inaccurately) the amount of 
bytes that header and footers are later turned into.

Since mod_logio lives in http/1.1 country, it is a bit tricky to report the 
bytes send for a h2 stream.

But there is an alternative way of getting that while keeping b->length == 0 
for H2HEADER buckets. So for simplicity alone, this should be changed.

- Stefan

Reply via email to