craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.25k stars 629 forks source link

[4.x]: 20+ DB writes from AssetManager per page load in the control panel #14374

Closed ericdrosas87 closed 7 months ago

ericdrosas87 commented 7 months ago

What happened?

Description

Upon logging into the Craft control panel, approximately 20 DB writes are made to the database by the AssetManager. Accordingly to @timkelty this should not be happening with each page request, though that is indeed what I am seeing.

We are hosting this in Google Cloud's Kubernetes Engine with Redis and the Google Cloud Storage plugin for assets and asset variants.

Apache info:

Server version: Apache/2.4.57 (Debian) Server built: 2023-04-13T03:26:51

Steps to reproduce

  1. Log into Craft control panel
  2. Enable debug panel for logged in user
  3. Open up the DB tab of the debug panel
  4. Navigate to a new control panel page to see the DB traffic

Alternatively for local dev:

  1. Add a Craft::info() log to the hash() function in craft\web\AssetManager to see the number of the upserts being performed with each request in the stdout log.
  2. tail -f the logs
  3. Navigate to different pages in the control panel to see the output in the logs, which for me looks like:
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @bower/jquery/dist {"memory":9891176} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @venveo/bulkedit/assetbundles/bulkeditelementaction/dist {"memory":9917872} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/cp/dist {"memory":9919904} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/tailwindreset/dist {"memory":9921888} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/axios/dist {"memory":9923904} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/d3/dist {"memory":9925872} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/elementresizedetector/dist {"memory":9927872} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/garnish/dist {"memory":9929920} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/jquerytouchevents/dist {"memory":9931904} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/velocity/dist {"memory":9933920} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/jqueryui/dist {"memory":9935936} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/jquerypayment/dist {"memory":9937952} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/picturefill/dist {"memory":9939968} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/selectize/dist {"memory":9941984} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/fileupload/dist {"memory":9944000} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/xregexp/dist {"memory":9946016} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/fabric/dist {"memory":9948000} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @craft/web/assets/iframeresizer/dist {"memory":9949984} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @modules/userregistrationmodule/assetbundles/userregistrationmodule/dist {"memory":9952032} 
2024-02-13 16:07:35 [web.INFO] [erosas] About to perform upsert for : @lsst/cantodamassets/web/assets/dist {"memory":9954112} 

These same upserts are seen with each page request and sometimes additional upserts are seen in the logs.

Expected behavior

Minimal if any DB writes per page request

Actual behavior

The same paths/hashes are passed to the upsert with almost every page request in the control panel.

Craft CMS version

4.5.4

PHP version

8.1

Operating system and version

Linux 5.15.65+

Database type and version

PostgreSQL 13.7

Image driver and version

Imagick 3.7.0 (ImageMagick 6.9.11-60)

Installed plugins and versions

-

    "require": {
        "carlcs/craft-assetmetadata": "^4.0",
        "castiron/next-builds": "^1.0",
        "craftcms/cms": "4.5.4",
        "craftcms/contact-form": "3.0.1",
        "craftcms/contact-form-honeypot": "^2.0.0",
        "craftcms/generator": "^1.6",
        "craftcms/google-cloud": "^2.0.0",
        "craftcms/redactor": "3.0.4",
        "jamesedmonston/graphql-authentication": "2.5.0",
        "lsst-epo/canto-dam-assets": "4.0.9",
        "rynpsc/craft-phone-number": "^2.1.0",
        "sebastianlenz/linkfield": "^2.1.4",
        "spicyweb/craft-neo": "3.8.6",
        "venveo/craft-bulkedit": "4.0.1",
        "verbb/super-table": "3.0.9",
        "vlucas/phpdotenv": "^3.4.0",
        "wrav/oembed": "^2.2.2"
    },
timkelty commented 7 months ago

Accordingly to @timkelty this should not be happening with each page request, though that is indeed what I am seeing.

@ericdrosas87 after going things over with @brandonkelly, I was mistaken and this is indeed expected. That said, these are extremely lightweight queries and unlikely to contribute meaningfully to performance.

Yii's asset bundles include a hash, and when load-balancing is in the mix, Craft has no way of knowing how to map the hash to an asset bundle source path, hence the existence of the resourcepaths.

We've considering moving this to be stored exclusively with the cache component, but that wouldn't necessarily be any faster, as in load-balanced envs, this usually means a network request of some sort (sounds like Redis in your case).

Configuring \craft\web\AssetManager::$cacheSourcePaths = false will prevent these writes, but will also cause CP assets to break in a multi-server/load-balanced env.

ericdrosas87 commented 7 months ago

I appreciate you looking into this @timkelty , I have a few follow-up questions:

Configuring \craft\web\AssetManager::$cacheSourcePaths = false will prevent these writes, but will also cause CP assets to break in a multi-server/load-balanced env.

So the control panel will still function as expected, but assets will be missing? Will Craft still serve up the graphQL /api endpoint data as expected?

Can you foresee any issues with only setting \craft\web\AssetManager::$cacheSourcePaths = false for the read-only regional stack we are setting up? By "read-only" I mean we don't anticipate content creators to use the Craft CMS in the region we are currently standing up, we just need it to serve up data from the read replica databases.

timkelty commented 7 months ago

So the control panel will still function as expected, but assets will be missing? Will Craft still serve up the graphQL /api endpoint data as expected?

CSS and JS for the control panel may be missing depending on which server handles those requests, and if they have the assets published to the cpresources directory.

The gql endpoint would be unaffected.

Can you foresee any issues with only setting \craft\web\AssetManager::$cacheSourcePaths = false for the read-only regional stack we are setting up?

Asset bundles are primarily used by the control panel, however, some plugins register them to inject into front end Twig templates. Those should be the only cases to be concerned with. Sounds like maybe you're running headless w/ GQL, so that may not even be a concern.

ericdrosas87 commented 7 months ago

Sounds like maybe you're running headless w/ GQL, so that may not even be a concern.

Yep we're running headless. We will give that a shot and I'll report back if we see any issues, thank you again!

timkelty commented 7 months ago

Sounds good!

ericdrosas87 commented 6 months ago

Sorry to dig this back up from the dead, but I would just like to confirm that the following code snippet will also work?:

    'components' => [
        'assetManager' => function() {
            # Get default config:
            $config = craft\helpers\App::assetManagerConfig();

            # Set custom property:
            $config['cacheSourcePaths'] = (bool)App::env('DISABLE_SOURCE_PATHS') ?: true;

            # Create + return component:
            return Craft::createObject($config);
        },
    // rest of configs

Specifically asking if setting the AssetManager config will work this way as well as the way you suggested (\craft\web\AssetManager::$cacheSourcePaths = false)

timkelty commented 6 months ago

@ericdrosas87 the closure looks right, but (bool)App::env('DISABLE_SOURCE_PATHS') ?: true will always evaluate to true.

Try replacing it with:

            $config['cacheSourcePaths'] = App::parseBooleanEnv('$DISABLE_SOURCE_PATHS') ?? true;
ericdrosas87 commented 6 months ago

@timkelty Good catch! Thank you!