Problem/Motivation

admin_toolbar_tools_update_8203() has this:

/**
 * Module CSS file refactoring requires cache rebuild.
 */
function admin_toolbar_tools_update_8203() {
  drupal_flush_all_caches();
}

This should never be done. Simply declaring an update hook is sufficient to ensure a cache rebuild on update; doing a cache rebuild inside it will cause caches to be cleared twice and can lead to data integrity problems.

Proposed resolution

Remove the cache clear from the update hook, leaving the hook itself in place. A cache clear will still be triggered simply by its presence for any sites that have not yet applied the update. (Keeping the existing hook is safe; just remove the cache clear from inside it.)

Core does this all the time with code like this:

/**
 * Disable blocks that are placed into the "disabled" region.
 */
function block_post_update_disabled_region_update() {
  // An empty update will flush caches, forcing block_rebuild() to run.
}

Remaining tasks

In the future, to clear the cache on update, write an empty post-update hook. See this change record for more information : https://www.drupal.org/node/2960601

User interface changes

API changes

Data model changes

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

xjm created an issue. See original summary.

xjm’s picture

Status: Active » Needs review

@Kingdutch is the one who originally identified this bug and brought it to my attention, so he should be credited on the issue as well.

xjm’s picture

Issue summary: View changes
xjm’s picture

Issue summary: View changes
klausi’s picture

Status: Needs review » Reviewed & tested by the community

Makes sense, thank you!

dydave made their first commit to this issue’s fork.

dydave’s picture

Status: Reviewed & tested by the community » Fixed

Thanks a lot Jess (@xjm) for reporting this issue and providing the code changes to fix it! 🙏

Sorry for the late reply on this. 😅

After reviewing the changes, following Klaus' (@klausi) review and since all the jobs and tests were still passing 🟢, I went ahead and merged the changes. 👍

Raaaaa ... I made a mistake with the commit message issue number ... a bad copy/paste from the previous commit and since I'm only developer in this repo, I'm unable to change it now 😖🔨
So the commits show here #3590488-10: Unused variable in SearchLinks.php.

At this point, since all the work to be carried in this ticket should have been completed, marking it as Fixed, for now.

Feel free to let us know if you have any questions or concerns on any of the recent code changes or the project in general, we would surely be glad to help.
Thanks again for your help keeping the module well maintained! 😊

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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