Add missing observers for invalidating cache upon reindex

[udovicic]

Add two missing events that were causing cache invalidation not to be triggered.

Example case would be saving category info with indexing mode set to 'on schedule' and 'flat tables' being turned on.

[daniel-ifrim]

@udovicic
I've looked on

<event name="clean_cache_after_reindex">
    <observer name="invalidate_varnish" instance="Fastly\Cdn\Observer\FlushAllCacheObserver"/>
</event>

and it's overkill.
After each run of reindex it flushes all cache from Fastly (all pages and everything ?).

Magento 2 by default has this in module Magento_PageCache:

<event name="clean_cache_after_reindex">
        <observer name="reindex_cache_flush" instance="Magento\PageCache\Observer\FlushCacheByTags" />
</event>

and in module Magento_CacheInvalidate:

<event name="clean_cache_after_reindex">
        <observer name="flush_varnish_pagecache" instance="Magento\CacheInvalidate\Observer\InvalidateVarnishObserver"/>
</event>

Magento 2, for built-in full page cache and Varnish cache, will clean cache by tags in those 2 Observers.
Adding flush everything in Fastly_Cdn after each reindex is run (every 1 minute) will flush all cache from Faslty.

Cleaning/purging all cache from Faslty after each product doesn't look great too:

<event name="controller_action_postdispatch_catalog_product_save">
    <observer name="invalidate_varnish" instance="Fastly\Cdn\Observer\FlushAllCacheObserver"/>
</event>

If there is something missing in Fastly_Cdn module, it should be coded in other manner. Maybe using like in Magento 2 default clean cache by tags observer: Fastly\Cdn\Observer\InvalidateVarnishObserver.

<event name="clean_cache_after_reindex">
    <observer name="invalidate_varnish" instance="Fastly\Cdn\Observer\InvalidateVarnishObserver"/>
</event>

I don't know about 'controller_action_postdispatch_catalog_product_save' event.

Also please take a look at this issue. It happens that we both are looking to same kind of issues:
#98

[daniel-ifrim]

@udovicic
About product save in admin.
In Magento_PageCache there is:

<event name="model_save_commit_after">
        <observer name="flush_cache_after_model_save" instance="Magento\PageCache\Observer\FlushCacheByTags" />
</event>

And with indexers set on Update by Schedule, I can confirm that rows are inserted in MView's changelog tables '[indexer_name]_cl'. Cron picks the changes and it will run indexers.
Some indexers call 'clean_cache_by_tags' event and mview dispatches again event 'clean_cache_after_reindex' in plugin
vendor/magento/module-indexer/Model/Processor/CleanCache.php, method afterUpdateMview

<type name="Magento\Indexer\Model\Processor">
        <plugin name="page-cache-indexer-reindex-clean-cache"
                type="Magento\Indexer\Model\Processor\CleanCache" sortOrder="10"/>
</type>

When indexers are set on Update on Save, I'm not particularly sure how exactly Varnish module Magento_CacheInvalidate cleans pages cache after product save in admin.
But there is this:
Magento\Framework\Model\AbstractModel::afterSave()
$this->_eventManager->dispatch('clean_cache_by_tags', ['object' => $this]);
And also Indexers run in cron:
Magento\Indexer\Cron\ReindexAllInvalid which goes to Magento\Indexer\Model\Processor\CleanCache::afterReindexAllInvalid and further in $this->eventManager->dispatch('clean_cache_by_tags', ['object' => $this->context]);.

Basically what I'm saying, in case of product save in admin, Magento 2 default code covers page cache cleaning (Fastly included too) with the existing events dispatched in the correct code context.
I may be missing something when indexers are set on Update on Save releted to Magento_CacheInvalidate module.

[udovicic]

Hi @daniel-ifrim,

You are correct,there is an room from improvement for cleaning cache. Let me look into those, and I will try to cover the #98 as well.

[vvuksan]

@daniel-ifrim can you verify if this addresses your concerns ?

[udovicic]

To share some more info on this, In my latest commit I have changed the way of purging cache, and now cache is purged by tags passed by the Magento framework (on same events). Therefore, cache that is still valid is preserved while the affected cache is purged.

[daniel-ifrim]

@udovicic @vvuksan

Please note that my feedback is based on Magento 2.1.0 - 2.1.7. I'm testing in 2.1.7 now.

<event name="clean_cache_after_reindex">
    <observer name="flush_fastly_cdn" instance="Fastly\Cdn\Observer\InvalidateVarnishObserver"/>
</event>

For 'clean_cache_after_reindex':

• Event 'clean_cache_after_reindex' has observer name "flush_varnish_pagecache" in Magento_CacheInvalidate/etc/events.xml.
  'invalidate_varnish' would be a new observer name and does not replace "flush_varnish_pagecache".
• Fastly_Cdn/etc/events.xml the observer name should be 'flush_fastly_cdn'. Unless you need to replace the observer from Magento_CacheInvalidate, in which case you would do name="flush_varnish_pagecache".

Adding an observer for event 'controller_action_postdispatch_catalog_product_save' is redundant with Magento 2 core code.

<event name="controller_action_postdispatch_catalog_product_save">
    <observer name="invalidate_varnish" instance="Fastly\Cdn\Observer\InvalidateVarnishObserver"/>
</event>

Magento 2 already cleans cache too many times on product save. See bellow why.

Most important is to add in Fastly_Cdn\etc\module.xml

<module name="Fastly_Cdn" ... >
    <sequence>
        <module name="Magento_Store"/>
        <module name="Magento_PageCache"/>
        <module name="Magento_CacheInvalidate"/> <!-- This is important for events.xml-->
    </sequence>
</module>

Because you replaced 'invalidate_varnish' of Magento_CacheInvalidate (default Varnish module):
Fastly_Cdn/etc/events.xml

<event name="clean_cache_by_tags">
    <observer name="invalidate_varnish" instance="Fastly\Cdn\Observer\InvalidateVarnishObserver"/>
</event>

Only that Magento_CacheInvalidate is after Fastly_Cdn in app/etc/config.php
It means that the etc/events.xml are merged, and the declaration from Fastly_Cdn module is overwritten by the one from Magento_CacheInvalidate.
So you need to add the module dependency in the Fastly_Cdn/etc/module.xml in order to have the observer from Fastly_Cdn to run, instead the one from Magento_CacheInvalidate.

Event 'clean_cache_by_tags' is important because it is the event responsible to clean full page cache selectively by cache tags. In case of Fastly_Cdn, the surrogate keys.
Additionally 'clean_cache_after_reindex' is run after each reindex of products:
vendor/magento/module-indexer/Model/Processor/CleanCache.php

I've tested in Magento 2.1.7, on product save 'clean_cache_by_tags' event is executed.
I've added a debug backtrace. It was a log message added in \Magento\PageCache\Observer\FlushCacheByTags::execute() after if (!empty($tags)) {.
Magento calls this event 6 times on simple product save in admin.

If you want to make tests about cleaning cache on product save in admin, I'would suggest to use a simple product.
Configurable products have a clean cache bug since 2.1.3:
magento/magento2#8009
I haven't looked yet in Magento 2.2.x or 2.1.8.

This is the backtrace log file:
https://justpaste.it/1ap3b

[udovicic]

Hi @daniel-ifrim, at first I have missed what you are saying. It was never my intention to override the observers, it was error in naming. I have adjusted. Also, the observer on admin action has been removed. Even though Magento core has it, it seems redundant after testing, so it was removed.

Everything was tested on 2.1.8 and 2.2.-dev and appears to be working as expected.

[daniel-ifrim]

@udovicic Thank you.