Skip to content
Snippets Groups Projects
Commit cbb8e240 authored by Christian Kuhn's avatar Christian Kuhn
Browse files

[TASK] Avoid runtime caching pages in DataHandler

The DataHandler has a major structural flaw by not handing
over known record state to sub methods. It thus tends to
fetch and process the same table/uid combination over and
over again to figure out details like permissions.

This is of course expensive: DH tends to fire lots of DB
calls. Over the years, a series of caches have been added
in order to suppress those DB calls and mitigate DB load.

Obvious cache layers:
* $this->recInsertAccessCache
* $this->isRecordInWebMount_Cache
* $this->isInWebMount_Cache
* $this->pageCache
* $this->runtimeCache 'checkRecordUpdateAccess_'
* $this->runtimeCache 'doesRecordExist_pageLookUp_'

As an additional gem that increases complexity, runtimeCache
is currently abused to transfer state between DH sub instances.
Various lower level caches kick in, too, e.g. from BackendUtility.

There is a problem with these caches, in particular those that
suppress record state DB calls: DH itself changes this state all
the time by adding, deleting and changing records. This makes
cache eviction a crucial aspect when dealing with them. This
however is hard and not properly done at various places. The true
solution is to hand over more 'known correct state at this time'
to sub methods instead, which voids more and more of these layers.
We've done this with the 'workspace discard' methods in v11 already
with great success: The DB load has been reduced significantly,
and there are no cache eviction issues in this area anymore.

The patch removes a cache in doesRecordExist_pageLookUp(),
which unblocks one aspect of a patch to fix #92604. For now,
we actively accept a (slight, but not critical) performance
penalty due to repeated DB queries, mainly hit when importing
many records. Looking at use cases, we can and should instead
refactor the call chains to hand over 'known' page record state
directly. We will establish this with upcoming v13 patches and
will suppress more queries than this cache avoided in the first
place.

Note the penalty introduced with this patch can be mostly
suppressed for imports using DH->bypassAccessCheckForRecords=true,
which is a good idea in many cases anyway at the moment, if
the import is an admin-only operation.

Resolves: #103172
Related: #79610
Related: #92604
Releases: main
Change-Id: Ie25cb4563c0bd09eb2d0060aafeae26bea7a54f5
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/83057


Tested-by: default avatarcore-ci <typo3@b13.com>
Tested-by: default avatarAnja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: default avatarSascha Nowak <typo3@saschanowak.me>
Reviewed-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: default avatarAnja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: default avatarOliver Klee <typo3-coding@oliverklee.de>
Reviewed-by: default avatarStefan Bürk <stefan@buerk.tech>
Tested-by: default avatarChristian Kuhn <lolli@schwarzbu.ch>
Tested-by: default avatarStefan Bürk <stefan@buerk.tech>
parent 6edfb0a8
Branches
Tags
No related merge requests found
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment