minor #22043 Refactor stale-while-revalidate code in HttpCache, add a (first?) test for it (mpdude)

This PR was squashed before being merged into the 3.3-dev branch (closes #22043).

Discussion
----------

Refactor stale-while-revalidate code in HttpCache, add a (first?) test for it

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

I came up with this while trying to hunt a production bug related to handling of stale cache entries under the condition of a busy backend (also see #22033).

It's just a refactoring to make the code more readable plus a new test.

Commits
-------

b14057c88a Refactor stale-while-revalidate code in HttpCache, add a (first?) test for it
This commit is contained in:
Fabien Potencier 2017-03-22 16:02:33 -07:00
commit 426a666def
3 changed files with 104 additions and 37 deletions

View File

@ -536,49 +536,39 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
// try to acquire a lock to call the backend
$lock = $this->store->lock($request);
if (true === $lock) {
// we have the lock, call the backend
return false;
}
// there is already another process calling the backend
if (true !== $lock) {
// check if we can serve the stale entry
if (null === $age = $entry->headers->getCacheControlDirective('stale-while-revalidate')) {
$age = $this->options['stale_while_revalidate'];
}
if (abs($entry->getTtl()) < $age) {
$this->record($request, 'stale-while-revalidate');
// server the stale response while there is a revalidation
return true;
}
// wait for the lock to be released
$wait = 0;
while ($this->store->isLocked($request) && $wait < 5000000) {
usleep(50000);
$wait += 50000;
}
if ($wait < 5000000) {
// replace the current entry with the fresh one
$new = $this->lookup($request);
$entry->headers = $new->headers;
$entry->setContent($new->getContent());
$entry->setStatusCode($new->getStatusCode());
$entry->setProtocolVersion($new->getProtocolVersion());
foreach ($new->headers->getCookies() as $cookie) {
$entry->headers->setCookie($cookie);
}
} else {
// backend is slow as hell, send a 503 response (to avoid the dog pile effect)
$entry->setStatusCode(503);
$entry->setContent('503 Service Unavailable');
$entry->headers->set('Retry-After', 10);
}
// May we serve a stale response?
if ($this->mayServeStaleWhileRevalidate($entry)) {
$this->record($request, 'stale-while-revalidate');
return true;
}
// we have the lock, call the backend
return false;
// wait for the lock to be released
if ($this->waitForLock($request)) {
// replace the current entry with the fresh one
$new = $this->lookup($request);
$entry->headers = $new->headers;
$entry->setContent($new->getContent());
$entry->setStatusCode($new->getStatusCode());
$entry->setProtocolVersion($new->getProtocolVersion());
foreach ($new->headers->getCookies() as $cookie) {
$entry->headers->setCookie($cookie);
}
} else {
// backend is slow as hell, send a 503 response (to avoid the dog pile effect)
$entry->setStatusCode(503);
$entry->setContent('503 Service Unavailable');
$entry->headers->set('Retry-After', 10);
}
return true;
}
/**
@ -707,4 +697,41 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
return $request->getMethod().' '.$path;
}
/**
* Checks whether the given (cached) response may be served as "stale" when a revalidation
* is currently in progress.
*
* @param Response $entry
*
* @return bool True when the stale response may be served, false otherwise.
*/
private function mayServeStaleWhileRevalidate(Response $entry)
{
$timeout = $entry->headers->getCacheControlDirective('stale-while-revalidate');
if ($timeout === null) {
$timeout = $this->options['stale_while_revalidate'];
}
return abs($entry->getTtl()) < $timeout;
}
/**
* Waits for the store to release a locked entry.
*
* @param Request $request The request to wait for
*
* @return bool True if the lock was released before the internal timeout was hit; false if the wait timeout was exceeded.
*/
private function waitForLock(Request $request)
{
$wait = 0;
while ($this->store->isLocked($request) && $wait < 5000000) {
usleep(50000);
$wait += 50000;
}
return $wait < 5000000;
}
}

View File

@ -562,6 +562,42 @@ class HttpCacheTest extends HttpCacheTestCase
$this->assertEquals('Hello World', $this->response->getContent());
}
public function testDegradationWhenCacheLocked()
{
$this->cacheConfig['stale_while_revalidate'] = 10;
// The prescence of Last-Modified makes this cacheable (because Response::isValidateable() then).
$this->setNextResponse(200, array('Cache-Control' => 'public, s-maxage=5', 'Last-Modified' => 'some while ago'), 'Old response');
$this->request('GET', '/'); // warm the cache
// Now, lock the cache
$concurrentRequest = Request::create('/', 'GET');
$this->store->lock($concurrentRequest);
/*
* After 10s, the cached response has become stale. Yet, we're still within the "stale_while_revalidate"
* timeout so we may serve the stale response.
*/
sleep(10);
$this->request('GET', '/');
$this->assertHttpKernelIsNotCalled();
$this->assertEquals(200, $this->response->getStatusCode());
$this->assertTraceContains('stale-while-revalidate');
$this->assertEquals('Old response', $this->response->getContent());
/*
* Another 10s later, stale_while_revalidate is over. Resort to serving the old response, but
* do so with a "server unavailable" message.
*/
sleep(10);
$this->request('GET', '/');
$this->assertHttpKernelIsNotCalled();
$this->assertEquals(503, $this->response->getStatusCode());
$this->assertEquals('Old response', $this->response->getContent());
}
public function testHitsCachedResponseWithSMaxAgeDirective()
{
$time = \DateTime::createFromFormat('U', time() - 5);

View File

@ -29,6 +29,10 @@ class HttpCacheTestCase extends TestCase
protected $responses;
protected $catch;
protected $esi;
/**
* @var Store
*/
protected $store;
protected function setUp()