Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 426a666

Browse filesBrowse files
fabpotfabpot
committed
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 ------- b14057c Refactor stale-while-revalidate code in HttpCache, add a (first?) test for it
2 parents 05933f3 + b14057c commit 426a666
Copy full SHA for 426a666

File tree

Expand file treeCollapse file tree

3 files changed

+103
-36
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+103
-36
lines changed

‎src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
+63-36Lines changed: 63 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -536,49 +536,39 @@ protected function lock(Request $request, Response $entry)
536536
// try to acquire a lock to call the backend
537537
$lock = $this->store->lock($request);
538538

539+
if (true === $lock) {
540+
// we have the lock, call the backend
541+
return false;
542+
}
543+
539544
// there is already another process calling the backend
540-
if (true !== $lock) {
541-
// check if we can serve the stale entry
542-
if (null === $age = $entry->headers->getCacheControlDirective('stale-while-revalidate')) {
543-
$age = $this->options['stale_while_revalidate'];
544-
}
545545

546-
if (abs($entry->getTtl()) < $age) {
547-
$this->record($request, 'stale-while-revalidate');
546+
// May we serve a stale response?
547+
if ($this->mayServeStaleWhileRevalidate($entry)) {
548+
$this->record($request, 'stale-while-revalidate');
548549

549-
// server the stale response while there is a revalidation
550-
return true;
551-
}
552-
553-
// wait for the lock to be released
554-
$wait = 0;
555-
while ($this->store->isLocked($request) && $wait < 5000000) {
556-
usleep(50000);
557-
$wait += 50000;
558-
}
550+
return true;
551+
}
559552

560-
if ($wait < 5000000) {
561-
// replace the current entry with the fresh one
562-
$new = $this->lookup($request);
563-
$entry->headers = $new->headers;
564-
$entry->setContent($new->getContent());
565-
$entry->setStatusCode($new->getStatusCode());
566-
$entry->setProtocolVersion($new->getProtocolVersion());
567-
foreach ($new->headers->getCookies() as $cookie) {
568-
$entry->headers->setCookie($cookie);
569-
}
570-
} else {
571-
// backend is slow as hell, send a 503 response (to avoid the dog pile effect)
572-
$entry->setStatusCode(503);
573-
$entry->setContent('503 Service Unavailable');
574-
$entry->headers->set('Retry-After', 10);
553+
// wait for the lock to be released
554+
if ($this->waitForLock($request)) {
555+
// replace the current entry with the fresh one
556+
$new = $this->lookup($request);
557+
$entry->headers = $new->headers;
558+
$entry->setContent($new->getContent());
559+
$entry->setStatusCode($new->getStatusCode());
560+
$entry->setProtocolVersion($new->getProtocolVersion());
561+
foreach ($new->headers->getCookies() as $cookie) {
562+
$entry->headers->setCookie($cookie);
575563
}
576-
577-
return true;
564+
} else {
565+
// backend is slow as hell, send a 503 response (to avoid the dog pile effect)
566+
$entry->setStatusCode(503);
567+
$entry->setContent('503 Service Unavailable');
568+
$entry->headers->set('Retry-After', 10);
578569
}
579570

580-
// we have the lock, call the backend
581-
return false;
571+
return true;
582572
}
583573

584574
/**
@@ -707,4 +697,41 @@ private function getTraceKey(Request $request)
707697

708698
return $request->getMethod().' '.$path;
709699
}
700+
701+
/**
702+
* Checks whether the given (cached) response may be served as "stale" when a revalidation
703+
* is currently in progress.
704+
*
705+
* @param Response $entry
706+
*
707+
* @return bool True when the stale response may be served, false otherwise.
708+
*/
709+
private function mayServeStaleWhileRevalidate(Response $entry)
710+
{
711+
$timeout = $entry->headers->getCacheControlDirective('stale-while-revalidate');
712+
713+
if ($timeout === null) {
714+
$timeout = $this->options['stale_while_revalidate'];
715+
}
716+
717+
return abs($entry->getTtl()) < $timeout;
718+
}
719+
720+
/**
721+
* Waits for the store to release a locked entry.
722+
*
723+
* @param Request $request The request to wait for
724+
*
725+
* @return bool True if the lock was released before the internal timeout was hit; false if the wait timeout was exceeded.
726+
*/
727+
private function waitForLock(Request $request)
728+
{
729+
$wait = 0;
730+
while ($this->store->isLocked($request) && $wait < 5000000) {
731+
usleep(50000);
732+
$wait += 50000;
733+
}
734+
735+
return $wait < 5000000;
736+
}
710737
}

‎src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php
+36Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,42 @@ public function testHitsCachedResponseWithMaxAgeDirective()
562562
$this->assertEquals('Hello World', $this->response->getContent());
563563
}
564564

565+
public function testDegradationWhenCacheLocked()
566+
{
567+
$this->cacheConfig['stale_while_revalidate'] = 10;
568+
569+
// The prescence of Last-Modified makes this cacheable (because Response::isValidateable() then).
570+
$this->setNextResponse(200, array('Cache-Control' => 'public, s-maxage=5', 'Last-Modified' => 'some while ago'), 'Old response');
571+
$this->request('GET', '/'); // warm the cache
572+
573+
// Now, lock the cache
574+
$concurrentRequest = Request::create('/', 'GET');
575+
$this->store->lock($concurrentRequest);
576+
577+
/*
578+
* After 10s, the cached response has become stale. Yet, we're still within the "stale_while_revalidate"
579+
* timeout so we may serve the stale response.
580+
*/
581+
sleep(10);
582+
583+
$this->request('GET', '/');
584+
$this->assertHttpKernelIsNotCalled();
585+
$this->assertEquals(200, $this->response->getStatusCode());
586+
$this->assertTraceContains('stale-while-revalidate');
587+
$this->assertEquals('Old response', $this->response->getContent());
588+
589+
/*
590+
* Another 10s later, stale_while_revalidate is over. Resort to serving the old response, but
591+
* do so with a "server unavailable" message.
592+
*/
593+
sleep(10);
594+
595+
$this->request('GET', '/');
596+
$this->assertHttpKernelIsNotCalled();
597+
$this->assertEquals(503, $this->response->getStatusCode());
598+
$this->assertEquals('Old response', $this->response->getContent());
599+
}
600+
565601
public function testHitsCachedResponseWithSMaxAgeDirective()
566602
{
567603
$time = \DateTime::createFromFormat('U', time() - 5);

‎src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,10 @@ class HttpCacheTestCase extends TestCase
2929
protected $responses;
3030
protected $catch;
3131
protected $esi;
32+
33+
/**
34+
* @var Store
35+
*/
3236
protected $store;
3337

3438
protected function setUp()

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.