On Fri, Jun 17, 2016 at 8:53 PM, Ruediger Pluem <rpl...@apache.org> wrote: > > > On 06/17/2016 05:55 PM, William A Rowe Jr wrote: >> A couple of branches/2.2.x STATUS questions are outstanding... >> >> *) mod_mem_cache: Don't cache incomplete responses when the client aborts >> the connection. >> Not applicable to trunk, mod_mem_cache doesn't exist there. >> 2.2.x patch: http://people.apache.org/~elu/mem_cache_client_abort.diff >> +1: elu, >> ylavic: don't we know from the very beginning of store_body() if >> r->connection->aborted, so to fail there? >> rpluem: Agreed. I guess we can move this up to line 733 before the loop. >> ylavic: Or even at the beginning if we want to avoid file caching too. >> But finally I wonder why we would not cache the response >> provided >> it is complete. Shouldn't incomplete response be detected with >> error buckets instead? ISTM that EOS with no error bucket *is* >> a complete response that could be cached even if the current >> client won't receive it completely. >> wrowe asks: Refreshed patch yet? Will give this a week before moving to >> the 'stalled' section below. >> >> What's the resolution here? Is there a patch yet that satisfies you both, >> that we can review? > > No that I am aware. Without having a patch at hand and time to create one > currently I > guess the correct condition would be to NOT cache if the connection is > aborted and there > is NO EOS bucket in the actual brigade. How to do this most effectively is > left as an exercise > for the reader :-). > But I guess the best approach is to test before the brigade iteration whether > the connection > is aborted and if it is if the brigade contains an EOS.
New patch ([1]) proposed in r1750225. Simply moving the "aborted" check after the loop(s) seems to be the simplest solution. If everything goes to EOS before I think we can cache the response (whether or not it will be sent to the client), but if the client's connection is aborted whereas the response is incomplete, we still discard the (about to be) truncated cache entry (like with Ed's original patch). Looks good to me... Regards, Yann. [1] http://home.apache.org/~ylavic/patches/httpd-2.2.x-mem_cache_client_abort-v2.patch
