Problem/Motivation

The User data service offers a public API to store additional data related to individual users. The related UserDataInterface exposes two methods that perform storage writes: set() and delete(). The default implementation does not take cache tag invalidation into account, which may cause stale cache entries not to be invalidated.

Proposed resolution

Add cache invalidation to UserData::set() and UserData::delete().

CommentFileSizeAuthor
#16 interdiff-3-16.txt1.53 KBhardik_patel_12
#16 user-data_cache_invalidate-2477903-16.patch2.38 KBhardik_patel_12
#3 user-data_cache_invalidate-2477903-3.patch1.64 KBplach
#1 user-data_cache_invalidate-2477903-1.patch2.27 KBplach

Issue fork drupal-2477903

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:

About issue forks

Comments

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Active » Needs review
Issue tags: +Needs tests
StatusFileSize
new user-data_cache_invalidate-2477903-1.patch2.27 KB

This is a quick patch I put together, it has the problem that we'd be invalidating cache tags each time ::set() is called, which would perform very bed when multiple properties need to be set. Not sure whether that's a common case, but we have such an example in User::preSave(). One possible fix could adding a parameter to skip cache tag invalidation (defaulting to FALSE).

Status: Needs review » Needs work

The last submitted patch, 1: user-data_cache_invalidate-2477903-1.patch, failed testing.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Status: Needs work » Needs review
StatusFileSize
new user-data_cache_invalidate-2477903-3.patch1.64 KB

Removed unrelated hunk

fabianx’s picture

fabianx commented

I was under the impression that at least in the mysql cache tags invalidator we have protection against multiple invalidations of the same tag in the same request.

If not however, this should be fixed in the invalidator and not here in my opinion.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

That's entirely possible :)

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/modules/user/src/UserData.php
@@ -119,4 +125,25 @@ public function delete($module = NULL, $uid = NULL, $name = NULL) {
+   * @param int $uids
+   *   The user account ID the data is associated with.

Its a bit odd that we allow multiple $uids, even the set method gets the $uid as singular.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

The delete method allows multiple ids to be specified, although the parameter is called $uid. If Fabian is correct about invalidating tags multiple times in the same request being ok, we should add the call also on delete.

fabianx’s picture

fabianx commented
Parent issue: » #2429287: [meta] Finalize the cache contexts API & DX/usage, enable a leap forward in performance

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Needs work 
  1. +++ b/core/modules/user/src/UserData.php
@@ -99,12 +101,16 @@ public function set($module, $uid, $name, $value) {
+    // We do not need to invalidate cache tags here as this is invoked in
+    // User::postDelete().

    I'm not sure this makes sense ... User::postDelete() is just executed when the entire user is removed. In this method you can also remove SOME of the user data.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

hardik_patel_12’s picture

hardik_patel_12 commented
Version: 8.6.x-dev » 8.9.x-dev
Status: Needs work » Needs review
StatusFileSize
new user-data_cache_invalidate-2477903-16.patch2.38 KB
new interdiff-3-16.txt1.53 KB

Re-rolled patch for D8.9.X and removing cache invalidation from testContactLink function.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

bramdriesen’s picture

bramdriesen
he/him
Primary language Dutch
Location Belgium 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

I tested this patch and it seems to work fine.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented 
+++ b/core/modules/user/src/UserData.php
@@ -89,12 +91,15 @@ public function set($module, $uid, $name, $value) {
    */
   public function delete($module = NULL, $uid = NULL, $name = NULL) {
+    // We do not need to invalidate cache tags here as this is invoked in
+    // User::postDelete().

What #10 said. There are also no tests yet.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

acbramley’s picture

acbramley commented
Issue tags: +Needs issue summary update, +Bug Smash Initiative

This issue came up as part of the BSI daily triage.

The issue summary needs an update to reflect the changes mentioned in #10.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue summary: View changes
Issue tags: -Needs issue summary update

Prashant.c made their first commit to this issue’s fork.

prashant.c’s picture

prashant.c
he/him
Primary language English
Location Dharamshala
commented

Took the changes from #16 to create a MR.

It seems still a lot of work is left to be done in this. Keeping in NW.

Thanks

Prashant.c opened merge request !5960

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.