Cache API.getSegmentMetadata better in Segment #18333
Conversation
add cache and lock to segment load
convert some variables
peterhashair
changed the title
|
@tsteur just testing, I saw on the goal page, the same problem we have last time appears, happens here as well. each graphic called the |
update segement to just cache
remove use import
peterhashair
added
not-in-changelog
|
Using lazy cache would be ideal for performance but be quite a bit of work to invalidate all the caches in the right moments which makes it very hard and a long job. I guess the idea with transient cache is that it may be an easy fix and we already make it maybe say 50% better compared to making it 90% better but a lot of work. Be good to double check if with a few segments this method is called quite often during one request. |
update segement cache ID
There was a problem hiding this comment.
Left a comment for a small improvement. Otherwise this should be good to merge.
Checked the changes on a dashboard with a couple of widgets. Without the changes here API.getSegmentsMetadata was called over 2000 times when loading the page. With the changes it was only twice 👍
Co-authored-by: Stefan Giehl <stefan@matomo.org>
| $cache = PiwikCache::getTransientCache(); | ||
|
|
||
| //covert cache id | ||
| $cacheId = 'API.getSegmentsMetadata.'.SettingsPiwik::getPiwikInstanceId() . '.' . implode(",", $this->idSites); |
There was a problem hiding this comment.
just fyi, no change needed. For regular caches we don't need the instance ID in there. That was for the get_lock mysql method only.
There was a problem hiding this comment.
@tsteur ah, this is already merged, should I update this in another PR
There was a problem hiding this comment.
@peterhashair no changed needed (mention this in the comment above). It's just for future when we work with caches again.
Description:
fixes: #17944
add cache and lock to segment load, using lazy cache
Review