On 17 Jan 2007, at 12:15, Giuliano Gavazzi wrote:
note that I am still calling store_headers, for it has side effects
(presumably the stuff you say it should do). But store_disk_headers
should not, unless it overwrites the headers when called again with
the correct size. But this is not the case with the original code,
and after my patch from above it would end up writing a header
which was the concatenation of the wrong and the right one...
well, I do not know if this is platform specific, but on MacOS 10.4 +
Case-sensitive Journaled HFS+ there is a problem in store_headers:
apr_off_t off=0;
rv = apr_file_seek(dobj->hfd, APR_SET, &off);
does not rewind if the file has been opened with APR_FOPEN_BUFFERED.
Now, I am not sure what buffered means here, but the file is on disk,
perhaps because apr_file_flush has been called at the end of the
first pass of store_headers.
Anyway, because of APR_FOPEN_BUFFERED apr_file_seek will call setptr
instead of lseek, and I presume that setptr will just rewind the (now
empty) buffer.
There is another potential problem in store_headers, if the headers
file is found open but not for writing, it is reopened, but not
rewind. Doesn't this imply append?
if(!(flags & APR_WRITE)) {
apr_file_close(dobj->hfd);
rv = apr_file_open(&dobj->hfd, dobj->hdrsfile,
APR_WRITE | APR_BINARY | APR_BUFFERED, 0, r->pool);
if (rv != APR_SUCCESS) {
dobj->hfd = NULL;
return rv;
}
One of the points of those patches are to solve the "thundering
herd" problem, simply described as when a frequently accessed
object is expired all accesses are served directly by your backend
until one access has completed successfully and the cache has been
able to store it. This is Bad if it causes your backend to grind
to a halt.
well, I will test with a sleep in the dynamic page and hammer it
with ab, as I am doing regularly for testing...
I have done some testing and I can see what you mean: it happens when
the backend response goes beyond the default 10s of conf->updtimeout.
To avoid this, the header is always written when the cache thinks
it should cache something. Other requests will find this header,
and if the size is unknown they will wait until it's updated with
the correct size, otherwise they will do read-while-caching and
return the contents as the file is being cached.
this explains why the file is flushed, or the other processes might
find an empty one.
As pointed above we should just either close and reopen the headers
file or open it without the APR_FOPEN_BUFFERED flag set. Then we can
let again store_disk_headers do its job regardless of the state of
the size.
g
