@tsteur opened this Pull Request on March 25th 2022 Member

Description:

fix DEV-2707

Since Matomo 4.7 we noticed that we often have thousands of general cache invalidations within a minute. Around 100 general cache invalidations within approx 4 seconds but depends on how many archives there are to be invalidated. It causes one general cache invalidation per site every time we call prepareArchive. Aka if you have thousands of sites, and run heaps of archives, and all sites have regular traffic, this can cause a huge massive amount of invalidations.

image

All these cache invalidations are also particularly a problem as we're constantly clearing the cache, and also constantly populate the tracker cache every ms as there are concurrent tracking requests happening.

The problem is coming from this backtrace which is called once per site when preparing an archive run. So this can be called very often this logic and cause a huge amount of invalidations.


<a href='/0'>#0</a>  Piwik\Tracker\Cache::clearCacheGeneral() called at [core/Archive/ArchiveInvalidator.php:332]
<a href='/1'>#1</a>  Piwik\Archive\ArchiveInvalidator->markArchivesAsInvalidated() called at [plugins/CoreAdminHome/API.php:166]
<a href='/2'>#2</a>  Piwik\Plugins\CoreAdminHome\API->invalidateArchivedReports() called at [core/CronArchive.php:928]
<a href='/3'>#3</a>  Piwik\CronArchive->invalidateWithSegments() called at [core/CronArchive.php:873]
<a href='/4'>#4</a>  Piwik\CronArchive->invalidateArchivedReportsForSitesThatNeedToBeArchivedAgainImpl() called at [core/CronArchive.php:808]
<a href='/5'>#5</a>  Piwik\CronArchive->Piwik\{closure}() called at [core/Tracker/Cache.php:302]
<a href='/6'>#6</a>  Piwik\Tracker\Cache::withDelegatedCacheClears() called at [core/CronArchive.php:809]
<a href='/7'>#7</a>  Piwik\CronArchive->invalidateArchivedReportsForSitesThatNeedToBeArchivedAgain() called at [core/CronArchive/QueueConsumer.php:182]
<a href='/8'>#8</a>  Piwik\CronArchive\QueueConsumer->getNextArchivesToProcess() called at [core/CronArchive.php:394]
<a href='/9'>#9</a>  Piwik\CronArchive->run() called at [core/CronArchive.php:280]
<a href='/10'>#10</a> Piwik\CronArchive->Piwik\{closure}() called at [core/Access.php:661]
<a href='/11'>#11</a> Piwik\Access::doAsSuperUser() called at [core/CronArchive.php:286]
<a href='/12'>#12</a> Piwik\CronArchive->main() called at [plugins/CoreConsole/Commands/CoreArchiver.php:32]
<a href='/13'>#13</a> Piwik\Plugins\CoreConsole\Commands\CoreArchiver->execute() called at [vendor/symfony/console/Symfony/Component/Console/Command/Command.php:257]
<a href='/14'>#14</a> Symfony\Component\Console\Command\Command->run() called at [vendor/symfony/console/Symfony/Component/Console/Application.php:874]
<a href='/15'>#15</a> Symfony\Component\Console\Application->doRunCommand() called at [vendor/symfony/console/Symfony/Component/Console/Application.php:195]
<a href='/16'>#16</a> Symfony\Component\Console\Application->doRun()

To improve the situation my thought was to invalidate the general cache only every 4 seconds. We could technically only clear the cache after marking all archives as invalid. However, this could take a while depending on the number of archives to invalidate and the number of sites.

If it takes a few minutes between marking an archive as invalid and calling clearGeneralCache, then we risk that the given date won't be re-archived later under circumstances since the tracker wouldn't know we have invalidated the archive and therefore it wouldn't call rememberToInvalidateArchivedReportsLater and then we wouldn't archive that date again under some circumstances. It's bit hard to explain but generally we need a general cache clear ideally immediately after marking an archiving as invalid.

My trade-off was that it's unlikely that the only tracking request for the given date happens during those 5 seconds. And for today periods we always archive the date again the next day after midnight so it shouldn't be an issue.

Any thoughts on this? I haven't fully tested it yet but wanted to check for thoughts first.

There could be other solutions like having the information to rememberToInvalidateArchivedReportsLater in a tracker site cache but thinking this can make things worse as we can't optimise it as well as this one and causes again a lot of invalidations.

Review

@sgiehl commented on March 26th 2022 Member

I actually wasn't aware of that special caching mechanism. But I had a closer look. As far as I have seen those options are actually only stored to the general cache, as the tracker uses the value to check if the archives need to be invalidated or not. If I did not miss anything that is actually only done for tracking requests with a date before today. See https://github.com/matomo-org/matomo/blob/3b00013819863472afe30cc04de49c4365c22d21/core/Tracker/Visit.php#L622-L628
Note: Maybe we could even change that to before yesterday, as we always rearchive yesterday nevertheless, don't we?

So for most websites tracking older visits is very unlikely. The cache of those options seems only to be used within the Tracker. So regarding clearing the cache in markArchivesAsInvalidated, I was wondering if we can't simply skip it if another period than day should be invalidated or if the invalidated date is today (or yesterday). As in that cases updating the cache won't have any effect at all.

Your solution could be used on top maybe to avoid too many invalidations when older dates are invalidated.
Hope that makes sense...

@tsteur commented on March 29th 2022 Member

That's a great point @sgiehl 💯 This should reduce the general cache invalidations already quite a bit possibly (although not sure we set the flag for today or not).

I've adjusted the logic to

  • Only clear general cache if the date is not for today (using the same logic as in ArchiveInvalidator)
  • And only clear cache every 4 seconds only if any of the previous dates actually needed an invalidation

The logic is bit complicated to understand maybe but should tweak the performance well. I haven't fully tested it yet but wanted to see if you have any thoughts on this first @sgiehl ?

@sgiehl commented on March 30th 2022 Member

@tsteur I spent some more thoughts on that. Actually I think we did that a lot more complex than actually needed.
Clearing the cache is actually only needed if an option for a remembered invalidation was removed from the database.
That is triggered here:
https://github.com/matomo-org/matomo/blob/5a3ae2d7ff17d1d6bd11ffa65f78101c296bd99e/core/Archive/ArchiveInvalidator.php#L318-L331

So if we would change the forgetRememberedArchivedReportsToInvalidate to return true if a record has been removed, we could only clear the cache if that happened at least once within the foreach.
That should limit the number of cache clears to the number of option records in the database.

@tsteur commented on March 31st 2022 Member

@sgiehl great point, I didn't notice 🚀 that made it a lot easier.

For now I also removed the code that would invalidate only every 4 seconds as it be hard to implement with the deleted flag. I would do it this way, and then see if it's still a problem or not. If it then is still a problem, would add some extra code to invalidate less often.

Because in https://github.com/matomo-org/matomo/blob/3b00013819863472afe30cc04de49c4365c22d21/core/Tracker/Visit.php#L622-L628 which you pointed out we don't invalidate for today anyway, I also removed the check for isToday which wasn't needed in the first place.

@tsteur commented on April 1st 2022 Member

@sgiehl I'll leave this up to you. Safest be probably to put it into 4.10 and we could always apply a patch for this on production.

@sgiehl commented on April 1st 2022 Member

ok. moved it to 4.10..

This Pull Request was closed on April 1st 2022
Powered by GitHub Issue Mirror