serf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bert Huijben <>
Subject RE: [PATCH] Avoid empty reads when refilling aggregate buckets withhold_open()
Date Mon, 20 Mar 2017 07:27:36 GMT
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.


Sent from Mail for Windows 10

From: Evgeny Kotkov
Sent: vrijdag 17 maart 2017 12:55
Subject: [PATCH] Avoid empty reads when refilling aggregate buckets withhold_open()


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_peek_eagain): New tests.
  (test_buckets): Run new tests.

Patch by: Evgeny Kotkov <evgeny.kotkov{_AT_}>

Evgeny Kotkov

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message