Hi Evgeny,
Feel free to commit the 2 H2 patches and the SERF_DEBUG patch. I’ll do a more
careful review after commit.
I will do a more careful review to this aggregate bucket change first, because
I had some bad experiences with unexpected side effects on earlier changes to
this bucket myself.
Just causing an extra read somewhere may cause indirect effects on other
buckets, etc.
In all optimizations I tried, I had to eventually remove them later… I hope
this one is different, but I’ll check.
Bert
Sent from Mail for Windows 10
From: Evgeny Kotkov
Sent: vrijdag 17 maart 2017 12:55
To: dev@serf.apache.org
Subject: [PATCH] Avoid empty reads when refilling aggregate buckets
withhold_open()
Hi,
Currently, some of the buckets (HTTP/2 and Brotli) use the callback set
with serf_bucket_aggregate_hold_open() to refill the contents on the fly.
However, read(), read_iovec(), readline() and peek() implementations for
such aggregate buckets will sometimes ignore the fact that the bucket
has been refilled in this way and thus, are going to generate unnecessary
empty reads.
This patch tweaks how the aggregate buckets behave after calling hold_open()
and eliminates such unwanted empty reads where it's possible.
Log message:
[[[
Avoid unwanted empty reads after an aggregate bucket has been refilled
with the hold_open() callback.
Some of the buckets (HTTP/2 and Brotli) use the hold_open() callback
to refill the contents on the fly, and it would be better if they could
return the new data immediately, instead of artificially delaying it
until the next read.
* buckets/aggregate_buckets.c
(read_aggregate, serf_aggregate_readline, serf_aggregate_peek):
Check if the hold_open() callback has refilled the bucket list,
continue if it did that with an APR_SUCCESS.
* test/test_buckets.c
(test_hold_open_action_t, test_hold_open_context_t, test_hold_open,
create_test_hold_open_context): New helpers that allow mocking the
behavior of a hold_open() callback.
(test_aggregate_bucket_hold_open_read_iovec,
test_aggregate_bucket_hold_open_readline,
test_aggregate_bucket_hold_open_peek_eagain): New tests.
(test_buckets): Run new tests.
Patch by: Evgeny Kotkov <evgeny.kotkov{_AT_}visualsvn.com>
]]]
Regards,
Evgeny Kotkov
