Problem/Motivation

Recently we had a problem releasing an update to our WxT-based codebase, which blew up when it tried to sync updated config on a cache-rebuild, before a hook_update() could install the new module which provides the dependencies for the updated config. It turned out to be wxt_core's preUpdate hook calling drupal_flush_all_caches(), which is what triggers the too-early config sync from disk (via config_enforce.

I've read the inline comment above that preUpdate() method, and I'm not sure I've seen the kind of errors it refers to. I think in general, if a particular hook_update() is in danger of getting stale data from a cache, it should handle that directly. I think there's at least an equal likelihood that rebuilding (from mismatched disk/db states) ALL caches prior to update is likely to cause problems beyond what I'm seeing with Config Enforce.

Perhaps this is a holdover from D7, when we were merely "clearing" caches? Or possibly Config Enforce is unique (for now) in using hook_rebuild() to actually rebuild things in the db? Honestly not sure.

Steps to reproduce

This may be overcomplicated, but it's the simplest version I can think of atm, from the actual use-case we had:

  1. Install a basic site on WxT, create a node type (nodeA) with a single field (fieldA). Use Config Enforce (Devel) to "enforce" these into a custom (target) module, targetA.
    • Config Enforce is simply managing config (similar to Features, of old) by keeping active config synchronized with disk. When something is "enforced" by config_enforce, what's on disk will take precedence, and a cache-rebuild triggers an import
    • Simulate a "release" the code at this point, so there is a "live" instance of the site with only targetA installed.
  2. Create a new custom (target) module, targetB. Tell Config Enforce to use it as a Target Module.
  3. Add a new content type, in targetB, and add a field to that one (fieldB). Ensure the nodetype and field settings are enforced in targetB module.
  4. Add an instance of fieldB to nodeA, and enforce it's settings in targetA.
  5. Ensure all of these new configs are enforced and committed to the codebase.
  6. Write a simple hook_update() to install targetB when this second "release" is deployed.
  7. We now have the situation where the live site:

  • Has only targetA installed and running, with nodeA and fieldA.
  • Will soon receive second release codebase, which contains updates to the form_display and other node-related configs, as well as the new targetB module
  • Needs targetB to be installed *before* the targetA config is sync'd with active storage, since it now contains dependencies on targetB's field.storage config for fieldB.

Continuing then, we simulate a deploy to the live site:

  • Roll back the codebase to the commit we called the first release
  • Restore a snapshot of the database at that time
  • Run drush updb

Observe that Config Enforce will be triggered to sync the config updates before the hook_update() runs, and thus leave the site in an unknown state.

Proposed resolution

I'd honestly like to better understand the need for the preUpdate hook in the first place, but assuming we can't just get rid of it entirely, I propose a simple patch to set the system maintenance_mode state, so that Config Enforce (or other hook_rebuild() mechanisms) can recognize they're being triggered during an update, and bail early.

Remaining tasks

Simple patch, coming shortly :)

Issue fork wxt-3248590

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

spiderman created an issue. See original summary.

natew’s picture

If you are using the wxt helm charts, you can "disable" the first drush cr before the drush updatedb for drupal 8/9.: https://github.com/drupalwxt/helm-drupal/blob/master/drupal/templates/jo...

spiderman’s picture

@natew That's helpful, but I'm not sure the helm chart will disable the preUpdate hook here: https://github.com/drupalwxt/wxt/blob/4.1.x/modules/custom/wxt_core/src/...

sylus’s picture

Version: 4.1.x-dev » 4.2.x-dev
Status: Active » Reviewed & tested by the community

I think this fix is reasonable so lets add it to the 9.2.x line and document the change.

I really appreciate you filing a detailed issue for this :D

spiderman’s picture

Great, thanks for the update! :)

  • sylus committed 7146f5b on 4.1.x authored by spiderman
    Issue #3248590: Flushing caches during preUpdate hook should set...
sylus’s picture

Status: Reviewed & tested by the community » Fixed

Committed and attributed thx!

  • sylus committed 7146f5b on 4.2.x authored by spiderman
    Issue #3248590: Flushing caches during preUpdate hook should set...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.