Re: annou...@httpd.apache.org [moderator] missing

2021-10-14 Thread Ruediger Pluem



On 10/14/21 7:18 PM, William A Rowe Jr wrote:
> Stefan,
> 
> thanks for RM'ing!
> 
> On Fri, Oct 8, 2021 at 3:10 AM ste...@eissing.org  wrote:
>>
>> The 2.4.51 announcement on annou...@httpd.apache.org did not make it, as 
>> seen in the mailing list archives.
>>
>> I have no idea why not. Any moderator of that list here?
> 
> Still no sign of it in httpd-announce, the www-announce did make it.
> Any chance you had used a different email sender to httpd? You could
> try a ping to announce-owner@. Sebb would you happen to have a list of
> the current httpd-announce moderators? Are any active PMC members

I am happy to moderate httpd-announce if this helps.

> interested in helping? (Due to the from-@apache restriction, it's
> comparatively spam-free.)
> 
> Also a post-release cleanup step to add,
> https://downloads.apache.org/httpd/ is growing further out of date.

I now cleaned https://dist.apache.org/repos/dist/release/httpd/ up, but we need 
to add
this to automation. Once we move the new binaries there I guess we can remove 
the old ones. Or should we keep the version before
at this point of time and only remove it when doing the next release?

Regards

Rüdiger


Re: annou...@httpd.apache.org [moderator] missing

2021-10-14 Thread William A Rowe Jr
Stefan,

thanks for RM'ing!

On Fri, Oct 8, 2021 at 3:10 AM ste...@eissing.org  wrote:
>
> The 2.4.51 announcement on annou...@httpd.apache.org did not make it, as seen 
> in the mailing list archives.
>
> I have no idea why not. Any moderator of that list here?

Still no sign of it in httpd-announce, the www-announce did make it.
Any chance you had used a different email sender to httpd? You could
try a ping to announce-owner@. Sebb would you happen to have a list of
the current httpd-announce moderators? Are any active PMC members
interested in helping? (Due to the from-@apache restriction, it's
comparatively spam-free.)

Also a post-release cleanup step to add,
https://downloads.apache.org/httpd/ is growing further out of date.
This should contain only the current release except during the release
transition. In the day it takes to copy to the mirrors, the new
release will also be picked up permanently by archive.apache.org. So
once the website links are all updated, the previous release should be
removed to invalidate stale links.


Fixed: apache/httpd#2061 (trunk - a982f8f)

2021-10-14 Thread Travis CI
Build Update for apache/httpd
-

Build: #2061
Status: Fixed

Duration: 20 mins and 46 secs
Commit: a982f8f (trunk)
Author: Stefan Eissing
Message:   *) mod_http2: disabling 400_20 again, seeing if 401_31 stays fixed.



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1894242 
13f79535-47bb-0310-9956-ffa450edef68

View the changeset: 
https://github.com/apache/httpd/compare/a32ff18e4e4c...a982f8fa4787

View the full build log and details: 
https://app.travis-ci.com/github/apache/httpd/builds/239833006?utm_medium=notification_source=email


--

You can unsubscribe from build emails from the apache/httpd repository going to 
https://app.travis-ci.com/account/preferences/unsubscribe?repository=16806660_medium=notification_source=email.
Or unsubscribe from *all* email updating your settings at 
https://app.travis-ci.com/account/preferences/unsubscribe?utm_medium=notification_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.



Broken: apache/httpd#2060 (trunk - a32ff18)

2021-10-14 Thread Travis CI
Build Update for apache/httpd
-

Build: #2060
Status: Broken

Duration: 22 mins and 15 secs
Commit: a32ff18 (trunk)
Author: Stefan Eissing
Message:   *) mod_http2: another travis test of the 400_20 and 401_31 tests 
that produce a timeout
 only on travis and never locally. sorry about the cycles.



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1894241 
13f79535-47bb-0310-9956-ffa450edef68

View the changeset: 
https://github.com/apache/httpd/compare/eb4bfff27e33...a32ff18e4e4c

View the full build log and details: 
https://app.travis-ci.com/github/apache/httpd/builds/239830997?utm_medium=notification_source=email


--

You can unsubscribe from build emails from the apache/httpd repository going to 
https://app.travis-ci.com/account/preferences/unsubscribe?repository=16806660_medium=notification_source=email.
Or unsubscribe from *all* email updating your settings at 
https://app.travis-ci.com/account/preferences/unsubscribe?utm_medium=notification_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.



Fixed: apache/httpd#2059 (trunk - eb4bfff)

2021-10-14 Thread Travis CI
Build Update for apache/httpd
-

Build: #2059
Status: Fixed

Duration: 13 mins and 13 secs
Commit: eb4bfff (trunk)
Author: Stefan Eissing
Message:   *) mod_http2: disabling the 2 test cases failing on travis is 
interim 
 to not stand in other peoples way.



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1894235 
13f79535-47bb-0310-9956-ffa450edef68

View the changeset: 
https://github.com/apache/httpd/compare/7891b6f69463...eb4bfff27e33

View the full build log and details: 
https://app.travis-ci.com/github/apache/httpd/builds/239826514?utm_medium=notification_source=email


--

You can unsubscribe from build emails from the apache/httpd repository going to 
https://app.travis-ci.com/account/preferences/unsubscribe?repository=16806660_medium=notification_source=email.
Or unsubscribe from *all* email updating your settings at 
https://app.travis-ci.com/account/preferences/unsubscribe?utm_medium=notification_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.



Still Failing: apache/httpd#2058 (trunk - 7891b6f)

2021-10-14 Thread Travis CI
Build Update for apache/httpd
-

Build: #2058
Status: Still Failing

Duration: 13 mins and 15 secs
Commit: 7891b6f (trunk)
Author: Stefan Eissing
Message:   *) mod_http2: when pollset signals output, resume a streams data
 in nghttp2 every time without checks that response body bytes
 are available. This resolves the situation that a stream may stall
 when 2 consecutive H2HEADER buckets are sent (e.g. 103+200).



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1894228 
13f79535-47bb-0310-9956-ffa450edef68

View the changeset: 
https://github.com/apache/httpd/compare/f23144a627f4...7891b6f69463

View the full build log and details: 
https://app.travis-ci.com/github/apache/httpd/builds/239821573?utm_medium=notification_source=email


--

You can unsubscribe from build emails from the apache/httpd repository going to 
https://app.travis-ci.com/account/preferences/unsubscribe?repository=16806660_medium=notification_source=email.
Or unsubscribe from *all* email updating your settings at 
https://app.travis-ci.com/account/preferences/unsubscribe?utm_medium=notification_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.



Still Failing: apache/httpd#2057 (trunk - f23144a)

2021-10-14 Thread Travis CI
Build Update for apache/httpd
-

Build: #2057
Status: Still Failing

Duration: 10 mins and 16 secs
Commit: f23144a (trunk)
Author: Stefan Eissing
Message:   *) mod_http2: H2HEADER buckets have the correct lenght of zero and no
 longer smuggle the contained field lengths in this field. Instead
 the bytes reportded to mod_logio are counted specifically.



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1894226 
13f79535-47bb-0310-9956-ffa450edef68

View the changeset: 
https://github.com/apache/httpd/compare/4fc7e978f032...f23144a627f4

View the full build log and details: 
https://app.travis-ci.com/github/apache/httpd/builds/239820216?utm_medium=notification_source=email


--

You can unsubscribe from build emails from the apache/httpd repository going to 
https://app.travis-ci.com/account/preferences/unsubscribe?repository=16806660_medium=notification_source=email.
Or unsubscribe from *all* email updating your settings at 
https://app.travis-ci.com/account/preferences/unsubscribe?utm_medium=notification_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.



Re: svn commit: r1894220 - in /httpd/httpd/trunk: modules/http2/h2_bucket_beam.c test/modules/http2/test_004_post.py test/modules/http2/test_400_push.py test/modules/http2/test_401_early_hints.py

2021-10-14 Thread ste...@eissing.org
It has been changed! h2 header buckets have now a length of 0.

> Am 14.10.2021 um 11:34 schrieb ste...@eissing.org:
> 
> 
> 
>> Am 14.10.2021 um 11:28 schrieb Ruediger Pluem :
>> 
>> 
>> 
>> On 10/14/21 11:20 AM, ste...@eissing.org wrote:
>>> 
>>> 
 Am 14.10.2021 um 11:17 schrieb Ruediger Pluem :
 
 
 
 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=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=1894219=1894220=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(>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



Re: svn commit: r1894220 - in /httpd/httpd/trunk: modules/http2/h2_bucket_beam.c test/modules/http2/test_004_post.py test/modules/http2/test_400_push.py test/modules/http2/test_401_early_hints.py

2021-10-14 Thread ste...@eissing.org



> Am 14.10.2021 um 11:28 schrieb Ruediger Pluem :
> 
> 
> 
> On 10/14/21 11:20 AM, ste...@eissing.org wrote:
>> 
>> 
>>> Am 14.10.2021 um 11:17 schrieb Ruediger Pluem :
>>> 
>>> 
>>> 
>>> 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=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=1894219=1894220=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(>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

Re: svn commit: r1894220 - in /httpd/httpd/trunk: modules/http2/h2_bucket_beam.c test/modules/http2/test_004_post.py test/modules/http2/test_400_push.py test/modules/http2/test_401_early_hints.py

2021-10-14 Thread Joe Orton
On Thu, Oct 14, 2021 at 11:17:11AM +0200, Ruediger Pluem wrote:
> On 10/14/21 10:59 AM, ic...@apache.org wrote:
> > -
> > +
> > +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(>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?

No, definitely not, all metadata buckets must have length = 0.




Re: svn commit: r1894163 [1/8] - in /httpd/httpd/trunk: ./ changes-entries/ modules/http2/ test/modules/http2/

2021-10-14 Thread Ruediger Pluem



On 10/14/21 11:26 AM, ste...@eissing.org wrote:
> 
> 
>> Am 14.10.2021 um 11:23 schrieb Ruediger Pluem :
>>
>>
>>
>> On 10/14/21 11:16 AM, ste...@eissing.org wrote:
>>>
>>>
 Am 14.10.2021 um 11:07 schrieb Ruediger Pluem :



 On 10/12/21 3:34 PM, ic...@apache.org wrote:
> Author: icing
> Date: Tue Oct 12 13:34:01 2021
> New Revision: 1894163
>>
>
> static apr_off_t bucket_mem_used(apr_bucket *b)
> {
> -if (APR_BUCKET_IS_FILE(b)) {
> +if (APR_BUCKET_IS_FILE(b) || bucket_is_mmap(b)) {

 MMaped buckets also consume physical memory once the content was read. Of 
 course the pages can be dropped from physical memory
 again if space is needed. Furthermore they consume address space in the 
 process, but I admit that this will unlikely cause
 any issues on 64 bit architectures. The only thing that can happen here is 
 that people complain about large virtual memory sizes
 of processes which has no real impact in this case.
>>>
>>> My (maybe faulty) reasoning here is: 
>>> HTTP/2 does not create any FILE or MMAP buckets. It just wants to transfer 
>>> them efficiently from c2 to c1. Instead of reading them and copying the 
>>> chunks, it apr_mmap_dup() or apr_file_setaside() their content for the 
>>> receiving bucket brigade. 
>>>
>>> This should (as I understand it) not duplicate any memory or use more 
>>> memory than in a HTTP/1.1 response (plus the usual h2 overhead).
>>
>> Agreed, but reading consumers need to take care that they do not read too 
>> much at once without dropping the mmap bucket(s) after
>> reading, but this is not mod_http2's responsibility.
>> This leaves only the address space issue, which should only matter on 32 bit 
>> architectures in certain cases. So I guess this could
>> stay as is until people from non 64 bit platforms complain :-)
> 
> Isn't that what "EnableMMAP off" is for?

>From my point of view not really. MMap does not work well with files on 
>certain network file systems like NFS if file sizes change
while the file is open. This could cause core dumps. Hence the possibility to 
switch it off.

Regards

Rüdiger


Broken: apache/httpd#2056 (trunk - 4fc7e97)

2021-10-14 Thread Travis CI
Build Update for apache/httpd
-

Build: #2056
Status: Broken

Duration: 13 mins and 54 secs
Commit: 4fc7e97 (trunk)
Author: Stefan Eissing
Message:   *) 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.



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1894220 
13f79535-47bb-0310-9956-ffa450edef68

View the changeset: 
https://github.com/apache/httpd/compare/959dabfd836c...4fc7e978f032

View the full build log and details: 
https://app.travis-ci.com/github/apache/httpd/builds/239815704?utm_medium=notification_source=email


--

You can unsubscribe from build emails from the apache/httpd repository going to 
https://app.travis-ci.com/account/preferences/unsubscribe?repository=16806660_medium=notification_source=email.
Or unsubscribe from *all* email updating your settings at 
https://app.travis-ci.com/account/preferences/unsubscribe?utm_medium=notification_source=email.
Or configure specific recipients for build notifications in your .travis.yml 
file. See https://docs.travis-ci.com/user/notifications.



Re: svn commit: r1894220 - in /httpd/httpd/trunk: modules/http2/h2_bucket_beam.c test/modules/http2/test_004_post.py test/modules/http2/test_400_push.py test/modules/http2/test_401_early_hints.py

2021-10-14 Thread Ruediger Pluem



On 10/14/21 11:20 AM, ste...@eissing.org wrote:
> 
> 
>> Am 14.10.2021 um 11:17 schrieb Ruediger Pluem :
>>
>>
>>
>> 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=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=1894219=1894220=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(>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.

Regards

Rüdiger



Re: svn commit: r1894163 [1/8] - in /httpd/httpd/trunk: ./ changes-entries/ modules/http2/ test/modules/http2/

2021-10-14 Thread ste...@eissing.org



> Am 14.10.2021 um 11:23 schrieb Ruediger Pluem :
> 
> 
> 
> On 10/14/21 11:16 AM, ste...@eissing.org wrote:
>> 
>> 
>>> Am 14.10.2021 um 11:07 schrieb Ruediger Pluem :
>>> 
>>> 
>>> 
>>> On 10/12/21 3:34 PM, ic...@apache.org wrote:
 Author: icing
 Date: Tue Oct 12 13:34:01 2021
 New Revision: 1894163
> 
 
 static apr_off_t bucket_mem_used(apr_bucket *b)
 {
 -if (APR_BUCKET_IS_FILE(b)) {
 +if (APR_BUCKET_IS_FILE(b) || bucket_is_mmap(b)) {
>>> 
>>> MMaped buckets also consume physical memory once the content was read. Of 
>>> course the pages can be dropped from physical memory
>>> again if space is needed. Furthermore they consume address space in the 
>>> process, but I admit that this will unlikely cause
>>> any issues on 64 bit architectures. The only thing that can happen here is 
>>> that people complain about large virtual memory sizes
>>> of processes which has no real impact in this case.
>> 
>> My (maybe faulty) reasoning here is: 
>> HTTP/2 does not create any FILE or MMAP buckets. It just wants to transfer 
>> them efficiently from c2 to c1. Instead of reading them and copying the 
>> chunks, it apr_mmap_dup() or apr_file_setaside() their content for the 
>> receiving bucket brigade. 
>> 
>> This should (as I understand it) not duplicate any memory or use more memory 
>> than in a HTTP/1.1 response (plus the usual h2 overhead).
> 
> Agreed, but reading consumers need to take care that they do not read too 
> much at once without dropping the mmap bucket(s) after
> reading, but this is not mod_http2's responsibility.
> This leaves only the address space issue, which should only matter on 32 bit 
> architectures in certain cases. So I guess this could
> stay as is until people from non 64 bit platforms complain :-)

Isn't that what "EnableMMAP off" is for?

> 
> 
> Regards
> 
> Rüdiger
> 



Re: svn commit: r1894163 [1/8] - in /httpd/httpd/trunk: ./ changes-entries/ modules/http2/ test/modules/http2/

2021-10-14 Thread Ruediger Pluem



On 10/14/21 11:16 AM, ste...@eissing.org wrote:
> 
> 
>> Am 14.10.2021 um 11:07 schrieb Ruediger Pluem :
>>
>>
>>
>> On 10/12/21 3:34 PM, ic...@apache.org wrote:
>>> Author: icing
>>> Date: Tue Oct 12 13:34:01 2021
>>> New Revision: 1894163

>>>
>>> static apr_off_t bucket_mem_used(apr_bucket *b)
>>> {
>>> -if (APR_BUCKET_IS_FILE(b)) {
>>> +if (APR_BUCKET_IS_FILE(b) || bucket_is_mmap(b)) {
>>
>> MMaped buckets also consume physical memory once the content was read. Of 
>> course the pages can be dropped from physical memory
>> again if space is needed. Furthermore they consume address space in the 
>> process, but I admit that this will unlikely cause
>> any issues on 64 bit architectures. The only thing that can happen here is 
>> that people complain about large virtual memory sizes
>> of processes which has no real impact in this case.
> 
> My (maybe faulty) reasoning here is: 
> HTTP/2 does not create any FILE or MMAP buckets. It just wants to transfer 
> them efficiently from c2 to c1. Instead of reading them and copying the 
> chunks, it apr_mmap_dup() or apr_file_setaside() their content for the 
> receiving bucket brigade. 
> 
> This should (as I understand it) not duplicate any memory or use more memory 
> than in a HTTP/1.1 response (plus the usual h2 overhead).

Agreed, but reading consumers need to take care that they do not read too much 
at once without dropping the mmap bucket(s) after
reading, but this is not mod_http2's responsibility.
This leaves only the address space issue, which should only matter on 32 bit 
architectures in certain cases. So I guess this could
stay as is until people from non 64 bit platforms complain :-)


Regards

Rüdiger



Re: svn commit: r1894220 - in /httpd/httpd/trunk: modules/http2/h2_bucket_beam.c test/modules/http2/test_004_post.py test/modules/http2/test_400_push.py test/modules/http2/test_401_early_hints.py

2021-10-14 Thread ste...@eissing.org



> Am 14.10.2021 um 11:17 schrieb Ruediger Pluem :
> 
> 
> 
> 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=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=1894219=1894220=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(>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.


>> -return APR_SUCCESS;
>> +goto cleanup;
>> }
>> -else if (APR_BUCKET_IS_FILE(b)) {
>> +/* non meta bucket */
>> +
>> +/* in case of indeterminate length, we need to read the bucket,
>> + * so that it transforms itself into something stable. */
>> +if (b->length == ((apr_size_t)-1)) {
>> +rv = apr_bucket_read(b, , , APR_BLOCK_READ);
>> +if (rv != APR_SUCCESS) goto cleanup;
>> +}
>> +
>> +if (APR_BUCKET_IS_FILE(b)) {
>> /* For file buckets the problem is their internal readpool that
>>  * is used on the first read to allocate buffer/mmap.
>>  * Since setting aside a file bucket will de-register the
> 
> Regards
> 
> Rüdiger



Re: svn commit: r1894220 - in /httpd/httpd/trunk: modules/http2/h2_bucket_beam.c test/modules/http2/test_004_post.py test/modules/http2/test_400_push.py test/modules/http2/test_401_early_hints.py

2021-10-14 Thread Ruediger Pluem



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=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=1894219=1894220=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(>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?


> -return APR_SUCCESS;
> +goto cleanup;
>  }
> -else if (APR_BUCKET_IS_FILE(b)) {
> +/* non meta bucket */
> +
> +/* in case of indeterminate length, we need to read the bucket,
> + * so that it transforms itself into something stable. */
> +if (b->length == ((apr_size_t)-1)) {
> +rv = apr_bucket_read(b, , , APR_BLOCK_READ);
> +if (rv != APR_SUCCESS) goto cleanup;
> +}
> +
> +if (APR_BUCKET_IS_FILE(b)) {
>  /* For file buckets the problem is their internal readpool that
>   * is used on the first read to allocate buffer/mmap.
>   * Since setting aside a file bucket will de-register the

Regards

Rüdiger



Re: svn commit: r1894163 [1/8] - in /httpd/httpd/trunk: ./ changes-entries/ modules/http2/ test/modules/http2/

2021-10-14 Thread ste...@eissing.org



> Am 14.10.2021 um 11:07 schrieb Ruediger Pluem :
> 
> 
> 
> On 10/12/21 3:34 PM, ic...@apache.org wrote:
>> Author: icing
>> Date: Tue Oct 12 13:34:01 2021
>> New Revision: 1894163
>> 
>> URL: http://svn.apache.org/viewvc?rev=1894163=rev
>> Log:
>>  *) mod_http2:
>> - Fixed an issue since 1.15.24 that "Server" headers in proxied requests
>>   were overwritten instead of preserved. [PR by @daum3ns]
>> - Added directove 'H2StreamTimeout' to configure a separate value for 
>> HTTP/2
>>   streams, overriding server's 'Timeout' configuration. [rpluem]
>> - HTTP/2 connections now use pollsets to monitor the status of the
>>   ongoing streams and their main connection when host OS allows this.
>> - Removed work-arounds for older versions of libnghttp2 and checking
>>   during configure that at least version 1.15.0 is present.
>> - The HTTP/2 connection state handler, based on an experiment and draft
>>   at the IETF http working group (abandoned for some time), has been 
>> removed.
>> - H2SerializeHeaders no longer has an effect. A warning is logged when 
>> it is
>>   set to "on". The switch enabled the internal writing of requests to be 
>> parsed
>>   by the internal HTTP/1.1 protocol handler and was introduced to avoid
>>   potential incompatibilities during the introduction of HTTP/2.
>> - Removed the abort/redo of tasks when mood swings lower the active 
>> limit.
>> 
>> 
>> Added:
>>httpd/httpd/trunk/changes-entries/http2_additions.txt
>>httpd/httpd/trunk/modules/http2/h2_c1.c
>>httpd/httpd/trunk/modules/http2/h2_c1.h
>>httpd/httpd/trunk/modules/http2/h2_c1_io.c
>>httpd/httpd/trunk/modules/http2/h2_c1_io.h
>>httpd/httpd/trunk/modules/http2/h2_c2.c
>>httpd/httpd/trunk/modules/http2/h2_c2.h
>>httpd/httpd/trunk/modules/http2/h2_c2_filter.c
>>httpd/httpd/trunk/modules/http2/h2_c2_filter.h
>>httpd/httpd/trunk/modules/http2/h2_conn_ctx.c
>>httpd/httpd/trunk/modules/http2/h2_conn_ctx.h
>>httpd/httpd/trunk/modules/http2/h2_protocol.c
>>httpd/httpd/trunk/modules/http2/h2_protocol.h
>> Removed:
>>httpd/httpd/trunk/modules/http2/h2_alt_svc.c
>>httpd/httpd/trunk/modules/http2/h2_alt_svc.h
>>httpd/httpd/trunk/modules/http2/h2_conn.c
>>httpd/httpd/trunk/modules/http2/h2_conn.h
>>httpd/httpd/trunk/modules/http2/h2_conn_io.c
>>httpd/httpd/trunk/modules/http2/h2_conn_io.h
>>httpd/httpd/trunk/modules/http2/h2_ctx.c
>>httpd/httpd/trunk/modules/http2/h2_ctx.h
>>httpd/httpd/trunk/modules/http2/h2_filter.c
>>httpd/httpd/trunk/modules/http2/h2_filter.h
>>httpd/httpd/trunk/modules/http2/h2_from_h1.c
>>httpd/httpd/trunk/modules/http2/h2_from_h1.h
>>httpd/httpd/trunk/modules/http2/h2_h2.c
>>httpd/httpd/trunk/modules/http2/h2_h2.h
>>httpd/httpd/trunk/modules/http2/h2_task.c
>>httpd/httpd/trunk/modules/http2/h2_task.h
>> Modified:
>>httpd/httpd/trunk/CMakeLists.txt
>>httpd/httpd/trunk/modules/http2/NWGNUmod_http2
>>httpd/httpd/trunk/modules/http2/config2.m4
>>httpd/httpd/trunk/modules/http2/h2.h
>>httpd/httpd/trunk/modules/http2/h2_bucket_beam.c
>>httpd/httpd/trunk/modules/http2/h2_bucket_beam.h
>>httpd/httpd/trunk/modules/http2/h2_config.c
>>httpd/httpd/trunk/modules/http2/h2_config.h
>>httpd/httpd/trunk/modules/http2/h2_headers.c
>>httpd/httpd/trunk/modules/http2/h2_mplx.c
>>httpd/httpd/trunk/modules/http2/h2_mplx.h
>>httpd/httpd/trunk/modules/http2/h2_proxy_session.c
>>httpd/httpd/trunk/modules/http2/h2_proxy_util.c
>>httpd/httpd/trunk/modules/http2/h2_proxy_util.h
>>httpd/httpd/trunk/modules/http2/h2_push.c
>>httpd/httpd/trunk/modules/http2/h2_request.c
>>httpd/httpd/trunk/modules/http2/h2_request.h
>>httpd/httpd/trunk/modules/http2/h2_session.c
>>httpd/httpd/trunk/modules/http2/h2_session.h
>>httpd/httpd/trunk/modules/http2/h2_stream.c
>>httpd/httpd/trunk/modules/http2/h2_stream.h
>>httpd/httpd/trunk/modules/http2/h2_switch.c
>>httpd/httpd/trunk/modules/http2/h2_util.c
>>httpd/httpd/trunk/modules/http2/h2_util.h
>>httpd/httpd/trunk/modules/http2/h2_version.h
>>httpd/httpd/trunk/modules/http2/h2_workers.c
>>httpd/httpd/trunk/modules/http2/h2_workers.h
>>httpd/httpd/trunk/modules/http2/mod_http2.c
>>httpd/httpd/trunk/modules/http2/mod_http2.dsp
>>httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
>>httpd/httpd/trunk/test/modules/http2/test_105_timeout.py
>>httpd/httpd/trunk/test/modules/http2/test_712_buffering.py
>> 
>> Modified: httpd/httpd/trunk/CMakeLists.txt
>> URL: 
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/CMakeLists.txt?rev=1894163=1894162=1894163=diff
>> ==
>> --- httpd/httpd/trunk/CMakeLists.txt (original)
>> +++ httpd/httpd/trunk/CMakeLists.txt Tue Oct 12 13:34:01 2021
>> @@ -469,18 +469,15 @@ SET(mod_http2_extra_defines  ssi

Re: svn commit: r1894163 [1/8] - in /httpd/httpd/trunk: ./ changes-entries/ modules/http2/ test/modules/http2/

2021-10-14 Thread Ruediger Pluem



On 10/12/21 3:34 PM, ic...@apache.org wrote:
> Author: icing
> Date: Tue Oct 12 13:34:01 2021
> New Revision: 1894163
> 
> URL: http://svn.apache.org/viewvc?rev=1894163=rev
> Log:
>   *) mod_http2:
>  - Fixed an issue since 1.15.24 that "Server" headers in proxied requests
>were overwritten instead of preserved. [PR by @daum3ns]
>  - Added directove 'H2StreamTimeout' to configure a separate value for 
> HTTP/2
>streams, overriding server's 'Timeout' configuration. [rpluem]
>  - HTTP/2 connections now use pollsets to monitor the status of the
>ongoing streams and their main connection when host OS allows this.
>  - Removed work-arounds for older versions of libnghttp2 and checking
>during configure that at least version 1.15.0 is present.
>  - The HTTP/2 connection state handler, based on an experiment and draft
>at the IETF http working group (abandoned for some time), has been 
> removed.
>  - H2SerializeHeaders no longer has an effect. A warning is logged when 
> it is
>set to "on". The switch enabled the internal writing of requests to be 
> parsed
>by the internal HTTP/1.1 protocol handler and was introduced to avoid
>potential incompatibilities during the introduction of HTTP/2.
>  - Removed the abort/redo of tasks when mood swings lower the active 
> limit.
> 
> 
> Added:
> httpd/httpd/trunk/changes-entries/http2_additions.txt
> httpd/httpd/trunk/modules/http2/h2_c1.c
> httpd/httpd/trunk/modules/http2/h2_c1.h
> httpd/httpd/trunk/modules/http2/h2_c1_io.c
> httpd/httpd/trunk/modules/http2/h2_c1_io.h
> httpd/httpd/trunk/modules/http2/h2_c2.c
> httpd/httpd/trunk/modules/http2/h2_c2.h
> httpd/httpd/trunk/modules/http2/h2_c2_filter.c
> httpd/httpd/trunk/modules/http2/h2_c2_filter.h
> httpd/httpd/trunk/modules/http2/h2_conn_ctx.c
> httpd/httpd/trunk/modules/http2/h2_conn_ctx.h
> httpd/httpd/trunk/modules/http2/h2_protocol.c
> httpd/httpd/trunk/modules/http2/h2_protocol.h
> Removed:
> httpd/httpd/trunk/modules/http2/h2_alt_svc.c
> httpd/httpd/trunk/modules/http2/h2_alt_svc.h
> httpd/httpd/trunk/modules/http2/h2_conn.c
> httpd/httpd/trunk/modules/http2/h2_conn.h
> httpd/httpd/trunk/modules/http2/h2_conn_io.c
> httpd/httpd/trunk/modules/http2/h2_conn_io.h
> httpd/httpd/trunk/modules/http2/h2_ctx.c
> httpd/httpd/trunk/modules/http2/h2_ctx.h
> httpd/httpd/trunk/modules/http2/h2_filter.c
> httpd/httpd/trunk/modules/http2/h2_filter.h
> httpd/httpd/trunk/modules/http2/h2_from_h1.c
> httpd/httpd/trunk/modules/http2/h2_from_h1.h
> httpd/httpd/trunk/modules/http2/h2_h2.c
> httpd/httpd/trunk/modules/http2/h2_h2.h
> httpd/httpd/trunk/modules/http2/h2_task.c
> httpd/httpd/trunk/modules/http2/h2_task.h
> Modified:
> httpd/httpd/trunk/CMakeLists.txt
> httpd/httpd/trunk/modules/http2/NWGNUmod_http2
> httpd/httpd/trunk/modules/http2/config2.m4
> httpd/httpd/trunk/modules/http2/h2.h
> httpd/httpd/trunk/modules/http2/h2_bucket_beam.c
> httpd/httpd/trunk/modules/http2/h2_bucket_beam.h
> httpd/httpd/trunk/modules/http2/h2_config.c
> httpd/httpd/trunk/modules/http2/h2_config.h
> httpd/httpd/trunk/modules/http2/h2_headers.c
> httpd/httpd/trunk/modules/http2/h2_mplx.c
> httpd/httpd/trunk/modules/http2/h2_mplx.h
> httpd/httpd/trunk/modules/http2/h2_proxy_session.c
> httpd/httpd/trunk/modules/http2/h2_proxy_util.c
> httpd/httpd/trunk/modules/http2/h2_proxy_util.h
> httpd/httpd/trunk/modules/http2/h2_push.c
> httpd/httpd/trunk/modules/http2/h2_request.c
> httpd/httpd/trunk/modules/http2/h2_request.h
> httpd/httpd/trunk/modules/http2/h2_session.c
> httpd/httpd/trunk/modules/http2/h2_session.h
> httpd/httpd/trunk/modules/http2/h2_stream.c
> httpd/httpd/trunk/modules/http2/h2_stream.h
> httpd/httpd/trunk/modules/http2/h2_switch.c
> httpd/httpd/trunk/modules/http2/h2_util.c
> httpd/httpd/trunk/modules/http2/h2_util.h
> httpd/httpd/trunk/modules/http2/h2_version.h
> httpd/httpd/trunk/modules/http2/h2_workers.c
> httpd/httpd/trunk/modules/http2/h2_workers.h
> httpd/httpd/trunk/modules/http2/mod_http2.c
> httpd/httpd/trunk/modules/http2/mod_http2.dsp
> httpd/httpd/trunk/modules/http2/mod_proxy_http2.c
> httpd/httpd/trunk/test/modules/http2/test_105_timeout.py
> httpd/httpd/trunk/test/modules/http2/test_712_buffering.py
> 
> Modified: httpd/httpd/trunk/CMakeLists.txt
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/CMakeLists.txt?rev=1894163=1894162=1894163=diff
> ==
> --- httpd/httpd/trunk/CMakeLists.txt (original)
> +++ httpd/httpd/trunk/CMakeLists.txt Tue Oct 12 13:34:01 2021
> @@ -469,18 +469,15 @@ SET(mod_http2_extra_defines  ssi
>  SET(mod_http2_extra_includes ${NGHTTP2_INCLUDE_DIR})
>