Bug #87139

Regression: getTreeList inserts duplicate keys in cache_treelist

Added by Alexander Schnitzler 9 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Should have
Category:
Caching
Target version:
Start date:
2018-12-12
Due date:
% Done:

100%

TYPO3 Version:
9
PHP Version:
7.2
Tags:
Complexity:
Is Regression:
Yes
Sprint Focus:

Description

Unfortunately https://review.typo3.org/58951/ did not actually solve issues 86028 and 86491 for good.

There are several issues with the last approach:

1) The expiration time of all created caches was 0, which resulted in a permanent creation and deletion of cache entries. This behaviour cannot be called caching.
2) Number 1 increases the chance for race conditions where several parallel requests tried to create the same cache entry.

The solution:

New cache entries get an expiry date of 2145909600 (that's equal PHP_INT_MAX (32bit) and the same number that is used in the caching framework).
Also, new caches are created within a transaction, which prevents duplicate key errors.


Related issues

Related to TYPO3 Core - Bug #86028: getTreeList inserts duplicate keys in cache_treelist Closed 2018-08-29
Related to TYPO3 Core - Bug #86491: Duplicate entry for PRIMARY key in cache_treelist Closed 2018-10-01

Associated revisions

Revision 190654d8 (diff)
Added by Alexander Schnitzler 9 months ago

[BUGFIX] getTreeList inserts duplicate keys in cache_treelist

Unfortunately https://review.typo3.org/58951/ did not actually
solve issues #86028 and #86491 for good.

There are two issues concerning the former approach:

1) The expiration time of all created caches was 0, which resulted
in a permanent creation and deletion of cache entries. This
behaviour cannot be called caching.

2) Number 1) increases the chance for race conditions where several
parallel requests tried to create the same cache entry.

To fix this, the check for an existing cache entry will be reverted
to behave like before the regression, i.e. cache entries with an
expiration timestamp of 0 are considered valid again.

Also, new caches are created within a transaction, which prevents
duplicate key errors.

Releases: master, 8.7
Resolves: #87139
Change-Id: If9470f6e0f875c0ec4fe3c092c9bd0dfc059de2d
Reviewed-on: https://review.typo3.org/59127
Reviewed-by: Markus Klein <>
Tested-by: TYPO3com <>
Reviewed-by: Benni Mack <>
Reviewed-by: Andreas Fernandez <>
Tested-by: Andreas Fernandez <>
Tested-by: Benni Mack <>

Revision 557e32ce (diff)
Added by Alexander Schnitzler 9 months ago

[BUGFIX] getTreeList inserts duplicate keys in cache_treelist

Unfortunately https://review.typo3.org/58951/ did not actually
solve issues #86028 and #86491 for good.

There are two issues concerning the former approach:

1) The expiration time of all created caches was 0, which resulted
in a permanent creation and deletion of cache entries. This
behaviour cannot be called caching.

2) Number 1) increases the chance for race conditions where several
parallel requests tried to create the same cache entry.

To fix this, the check for an existing cache entry will be reverted
to behave like before the regression, i.e. cache entries with an
expiration timestamp of 0 are considered valid again.

Also, new caches are created within a transaction, which prevents
duplicate key errors.

Releases: master, 8.7
Resolves: #87139
Change-Id: If9470f6e0f875c0ec4fe3c092c9bd0dfc059de2d
Reviewed-on: https://review.typo3.org/59139
Tested-by: TYPO3com <>
Reviewed-by: Benni Mack <>
Tested-by: Benni Mack <>

History

#1 Updated by Alexander Schnitzler 9 months ago

  • Related to Bug #86028: getTreeList inserts duplicate keys in cache_treelist added

#2 Updated by Alexander Schnitzler 9 months ago

  • Related to Bug #86491: Duplicate entry for PRIMARY key in cache_treelist added

#3 Updated by Gerrit Code Review 9 months ago

  • Status changed from Accepted to Under Review

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

#4 Updated by Gerrit Code Review 9 months ago

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/59127

#5 Updated by Markus Klein 9 months ago

  • Description updated (diff)

#6 Updated by Markus Klein 9 months ago

To reduce the load on the server while massive number of requests do the same work to generate the cache content, I propose to employ locking instead of transactions. Paradigm: Do work only once.

Basic scheme:

  • try cache
  • when hit, return
  • when miss, then acquire lock
  • try cache again (might have been filled meanwhile)
  • when hit, release lock, return
  • when miss, generate content
  • store cache entry, release lock, return

#7 Updated by Gerrit Code Review 9 months 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/59127

#8 Updated by Gerrit Code Review 9 months 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/59127

#9 Updated by Gerrit Code Review 9 months ago

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

#10 Updated by Gerrit Code Review 9 months ago

Patch set 1 for branch TYPO3_8-7 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/59139

#11 Updated by Anonymous 9 months ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#12 Updated by Benni Mack 5 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF