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

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

plach created an issue. See original summary.

plach’s picture

Assigned: plach » Unassigned
Status: Active » Needs review
catch’s picture

Status: Needs review » Needs work

Overall this looks good to me, but the post update has a merge conflict.

plach’s picture

Status: Needs work » Needs review

Rebased

catch’s picture

Status: Needs review » Reviewed & tested by the community

Looks great, can't see anything to complain about.

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

japerry’s picture

Status: Reviewed & tested by the community » Needs work

Changing $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.

plach’s picture

Good call. Maybe we could implement a tiny wrapper implementing StateInterface and 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?

catch’s picture

plach’s picture

Well, that wouldn't work with $this->state instanceof StateInterface checks :)

plach’s picture

Status: Needs work » Needs review

This is a lot of BC code, but at least it should be comprehensive ;)

japerry’s picture

Unfortunately, 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!

plach’s picture

Status: Needs review » Reviewed & tested by the community

Looks good and seems to be working well here, thanks :)

  • japerry committed 81c331cf on 8.x-3.x authored by plach
    Issue #3635585: Store the queue statistics in key-value storage instead...
japerry’s picture

Status: Reviewed & tested by the community » Fixed

Fixed! Gonna roll a release now

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.

  • japerry committed 8731af1b on 8.x-3.x
    Issue #3635585 follow-up: Add return types to StatsTrackerState::get()...
plach’s picture

@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.