Generated with the help of an LLM.
Problem/Motivation
The queue statistics are stored in state, and they are written at the end of every request that queues or processes invalidations, that is on every content save. Since Drupal 10.3, state is a cache collector, so each write invalidates the cached state entry that every request loads, and the site has to build it again.
On a busy editorial site, we saw this cache entry rebuilt every few minutes during editing hours. The statistics are only counters for the UI and Drush, so they do not need to be in state.
Proposed resolution
Store the statistics in a key-value collection.
Remaining tasks
- Write the MR.
- Review.
User interface changes
None.
API changes
The StatsTracker constructor takes KeyValueFactoryInterface instead of StateInterface. Subclasses that call the parent constructor must be updated. To keep backward compatibility, the constructor also accepts StateInterface with a deprecation message.
Data model changes
The five statistics move from the state collection to the purge.queue.stats key-value collection.
Issue fork purge-3635585
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
Comment #3
plachComment #4
catchOverall this looks good to me, but the post update has a merge conflict.
Comment #5
plachRebased
Comment #6
catchLooks great, can't see anything to complain about.
Comment #8
japerryChanging $state to $storage would potentially cause modules extending the class to fail. While its unlikely, it does break BC policy, so I'm not inclined to commit this as-is. The other options aren't great either, but maybe keep the $state var (it does have the same methods as KeyValueStoreInterface) and throw a deprecation msg.
If we can discuss this week before the D12 release goes out, I'd love to get this in.
Comment #9
plachGood call. Maybe we could implement a tiny wrapper implementing
StateInterfaceand wrapping the KV store, just to be safe? We could instantiate it in the constructor, assign it to$state, and throw deprecation messages whenever a method is accessed?Comment #10
catchAnother option would be to use https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Dependenc...
Comment #11
plachWell, that wouldn't work with
$this->state instanceof StateInterfacechecks :)Comment #12
plachThis is a lot of BC code, but at least it should be comprehensive ;)
Comment #13
japerryUnfortunately, this module spans enough different use-cases and submodules, its better safe than sorry. Its also why a new major version of purge is unlikely (the old version would have to stay around for quite a while).
The BC change is good, I added another commit because you cannot call $storage directly, in case it was not subclassed. Otherwise looks good to go!
Comment #14
plachLooks good and seems to be working well here, thanks :)
Comment #16
japerryFixed! Gonna roll a release now
Comment #19
plach@japerry
I realized the same issue affects
RuntimeMeasurementTracker: I just created #3637078: Store the purger runtime measurements in key-value storage instead of state to track that and posted a very similar MR there.