Project

General

Profile

Actions

Bug #88250

closed

Remove temporary page cache entry (aka page is being generated message)

Added by Helmut Hummel over 5 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Should have
Assignee:
-
Category:
-
Target version:
-
Start date:
2019-04-30
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

With little effort and a little concurrency it is still possible to trigger the TYPO3 frontend to show a "Page is being generated" message instead of the rendered content of the page.
This happens despite locking that was introduced to get rid of this message.

In general this message appears, when a page that should be cached is hit with empty caches and a second request hits the same page concurrently a few milliseconds later.
The second request will show the message (other than I would have expected to wait until the first page generated the cache).

This happens for the following reasons:

The temporary cache entry is still generated before actual page rendering starts. When page rendering finishes, this temporary cache entry is overwritten with the actual rendered page
A cached page consists of two separate cache entries in two separate caches. A TypoScript cache stored in cache_pagesection and with only the page id as identifier and the page cache stored in cache_pages with a lot of things being part of the identifier like page id, cHash, language...

This means the TypoScript cache is generated only once for a page, while there typically exist multiple page cache entries.

Both caches are protected from being generated multiple times by different requests with locking. The locking itself works fine as far as I investigated. However how the locking of both cache generations and the generation of the temporary cache entry is intertwined, leads to the (to me) unexpected behavior.

The code flow for the first hit with empty caches is roughly as follows:

  1. Lock TS cache generation (if empty cache)
  2. Lock page generation (if empty cache)
  3. Generate TS cache
  4. Generate temporary cache entry
  5. Release TS lock
  6. Generate page and (over-)write page cache
  7. Release TS lock
  8. Release page generation lock

The culprit is position 5, which was introduced in TYPO3 7.6. Before that version, both locks were only removed after complete page rendering finished.
The problem with that is, that TS lock is released after the temporary cache entry is written

It means a second request that is fired right after the first, will wait on position 1 until the TS lock is released and directly asks for a page cache entry, which is then found; the temporary cache entry.

This means it only need two concurrent requests on empty caches for the second request to show the page is being generated message.
Two requests do not mean a "high load" or "high traffic". It imho is a severe and esay to trigger bug that needs to be fixed, at best down to TYPO3 7.6.

The fix is easy: Remove position 5, or at least move it before generating the temporary cache entry. Both options lead to the same result on empty caches, because it does not matter whether the second request waits for the TS lock or the page lock to be released.

For master, we should entirely get rid of the temporary content, as it is not required any more. In older TYPO3 versions, the code could have proceeded without locking. If locking failed, it was silently ignored. With introduction of the new locking API in TYPO3 7.6 this is not the case any more. If locking fails, TYPO3 fails hard as well. So the poor mans mitigation for concurrent requests is not needed any more.


Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Bug #87980: TYPO3 can not handle two concurrent requests to the same (cachable) page without a 503 responseClosed2019-03-22

Actions
Actions #1

Updated by Helmut Hummel over 5 years ago

  • Related to Bug #87980: TYPO3 can not handle two concurrent requests to the same (cachable) page without a 503 response added
Actions #2

Updated by Gerrit Code Review over 5 years ago

  • Status changed from New to Under Review

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60634

Actions #3

Updated by Gerrit Code Review over 5 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60634

Actions #4

Updated by Gerrit Code Review over 5 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/60634

Actions #5

Updated by Helmut Hummel over 5 years ago

  • Status changed from Under Review to Closed

I missed some important detail in my analysis.

That is, that no lock is acquired, when a cache entry is found.
This means, that once the first request that generates the page will have written the pagesection cache AND the temporary cache,
all following requests will see the "page is being generated" message until the first request finishes rendering.

So there is no other fix for this, than NOT writing the temporary cache entry.

I therefore close this ticket in favor of #87980

Actions

Also available in: Atom PDF