[RFC] purge on kernel terminate, instead of post flush

[bendavies]

Q	A
Bug fix?	no
New feature?	no
BC breaks?	yes? public signature? But class is @experimental
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR

It seems wrong to me to do purging in post flush, as this causes wait time for the user.
kernel.terminate seems the right place for this, as it was designed for heavy work after the user has their response.

Additionally, currently, if a purge fails, the user will receive a 500, which seems wrong, as their request actually succeeded (writing to the api).

[Simperfit]

It seems strange to me, you are now purging on each terminate?

[Simperfit]

If you do that you are not in the right place. This is in the doctrine bridge

[bendavies]

indeed good point. there would be a better place if the change is thought to be good.

[bendavies]

@Simperfit what do you find strange now? why would you not purge on terminate? This is where the far more mature https://github.com/FriendsOfSymfony/FOSHttpCacheBundle does it

[dunglas]

My first implementation was using kernel.terminate by I've switched to the current behavior thanks to @jderusse's review (#952).

Using kernel.terminate increased the risk of race conditions and prevents from displaying errors to the user.

[jderusse]

An optimization could be to call purge in an asynchronous way, and to wait the sync just before the kernel.terminate.

[bendavies]

Thanks @dunglas . For reference @jderusse's original comment: #952 (comment)

As far as i can see, there is no race condition here? I can't see an eventuality where the wrong data will be served by varnish for any considerable length of time.
This seems more like an Eventual Consistency problem to me. By suggesting moving the purge to postFlush @jderusse, was trying to minimise the time that varnish could be serving stale content.

Using both postFlush and kernel.terminate will both stop varnish serving stale content, but kernel.terminate doesn't penalise the current request's user with waiting for the sync purge to happen.
Of course, using kernel.terminate slightly increases the time that varnish could be serving stale content, but really how much time? milliseconds?

Secondly, @dunglas, why do you think serving a user an error if a varnish purge fails is a good thing? There is nothing wrong with their request - it has persisted to the database just just fine. It's an implementation detail that the platform is purging varnish. The user doesn't care. They don't want to know if it fails. If it fails, maybe the platform should be retrying the purge in some way, again, async and transparent to the user.

@dbu, might you have some valuable insight?

[jderusse]

Of course, using kernel.terminate slightly increases the time that varnish could be serving stale content, but really how much time? milliseconds?

In summary the topic is to decreasing consistency to gain milliseconds on writes?

If yes, I suggest to run the write queries in async way for your client (ajax/guzzle/whatever), and let the choice to user who want consitency (like a TestSuite) to wait the end of the complete transaction.

[dbu]

hi. i don't know all of the context, so i can just offer some random thoughts.

• naming: you call this "purger" - if i understand correctly, this is about tags, so it will end up as BAN requests. in FOSHttpCache, we only call something purge when it can be done as a single request to remove that request (in all variants) from varnish. the thing is that bans add to the banlist of varnish, and if there is a lot of bans and a lot of traffic, varnish can run into troubles.
• in FOSHttpCache 2 we have the ResponseTagger to record tags, and CacheInvalidator to invalidate tags. in FOSHttpCacheBundle, there is a listener for tags configured on routes, and the invalidation listener that flushes the CacheInvalidator on kernel terminate. There is however no doctrine integration atm. If we would have one, i would it have collect tags to invalidate but only flush them in terminate.

i guess the question here is how bad serving outdated information is. we recently had a situation with our api where we send message queue notifications of changes and because the consumers were too fast, they fetched the content before the cache was invalidated. we refactored our system to guarantee that invalidation is done before the notification is sent. but that comes at a real cost of making the requests that change data slower. on further reflection, we realized that the consumers of such messages (in our case its only a single consumer for each resource) only need the notification because they maintain some sort of cache of their own (in the end, the notification is kindof like a cache invalidation too...). so the most efficient way to handle this imho is for those consumers to send a request that bypasses the cache (either directly to symfony, or with a no-cache request header that varnish would respect).

i am sure there are other scenarios however, where this approach does not work out. but i would assume that the default case is where a slight delay in invalidation is not a problem. i would probably rather explain this somewhere in the caching documentations, and mention that if you send out notifications and need to guarantee that the cache is invalidated when they are sent, you should at that point manually flush the CacheInvalidator.

as for getting an error when invalidation fails, i also think as client of the API, i don't want to have to care about this.

[soyuka]

closing as old, lmk if I should reopen