Comment in code says:
Test for the page_cache module, which is required for cache invalidation.

This is not valid. Page cache module is not needed when having an external caching solution.

Warning says on status page:
Please install the page_cache module. The page cache acts as a 'second layer of defence' by keeping copies of all generated pages, which protects you from widespread performance degradation in case of emergencies.

This should be bad practice, caching a lot of objects that you do not need to cache, if, for instance, varnish has problems, this will not help at all in a high traffic situation. It could do things even worse, if using db for cache, also the warning could make devs believe that Page cache module is needed for make purging work.

Suggestion:
Remove PageCacheCheck.php

CommentFileSizeAuthor
#24 dummy-page_cache-plugin-to-prevent-errors-on-deploy-2803607.patch826 bytesralphvdhoudt
#18 purge-2803607-18.patch2.54 KBkrzysztof domański
#14 Remove-PageCacheCheck-2803607-14.patch2.54 KBjjcarrion
#9 Remove-PageCacheCheck-2803607-9.patch2.53 KBjjcarrion
#3 Remove-PageCacheCheck-2803607-2.patch2.72 KBmisc

Comments

MiSc created an issue. See original summary.

misc’s picture

misc commented
Issue summary: View changes
misc’s picture

misc commented
StatusFileSize
new Remove-PageCacheCheck-2803607-2.patch2.72 KB
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Suggestion - remove PageCacheCheck » Suggestion - remove PageCacheCheck, or document its recommendation very clearly
Category: Feature request » Task
Priority: Minor » Normal

If the maintainer decides to keep this, then they should at least document very clearly/explicitly why they think this is necessary.

misc’s picture

misc commented
Status: Active » Needs review
caspervoogt’s picture

caspervoogt commented

I agree with this suggestion.

dragonwize’s picture

dragonwize commented

The description of the page_cache module in core even reads:

Caches pages for anonymous users. Use when an external page cache is not available.

moshe weitzman’s picture

moshe weitzman commented

+1 to removing this warning. Its a bit too opinionated IMO.

jjcarrion’s picture

jjcarrion
Primary language Spanish
Location Spain
commented
Assigned: misc » Unassigned
StatusFileSize
new Remove-PageCacheCheck-2803607-9.patch2.53 KB

Reroll to work with 3.0.0-beta9

gaëlg’s picture

gaëlg
he/him
Primary language French
Location Lille, France
commented
Status: Needs review » Reviewed & tested by the community

If I understand well the guidelines (https://www.drupal.org/node/156119), it's RTBC, right?

mpp’s picture

mpp commented

RTBC++

"The page cache acts as a 'second layer of defence' by keeping copies of all generated pages, which protects you from widespread performance degradation in case of emergencies." => as mentioned in the issue description there's no use for page_cache when using an external cache.

lisa.rae’s picture

lisa.rae commented
Status: Reviewed & tested by the community » Needs work

Marking Needs Work, patch is failing testing.

misc’s picture

misc commented

It is not just the pull request that is falling testing, it is also the main main 8.x-3.x-dev that is failing:
https://www.drupal.org/pift-ci-job/1595419

jjcarrion’s picture

jjcarrion
Primary language Spanish
Location Spain
commented
Status: Needs work » Needs review
StatusFileSize
new Remove-PageCacheCheck-2803607-14.patch2.54 KB

Reroll to work with 3.0.0

mpp’s picture

mpp commented
Status: Needs review » Reviewed & tested by the community

Thanks for the reroll, setting back to rtbc.

zero2one’s picture

zero2one commented

RTBC

sime’s picture

sime
any/any
Location Melbourne
commented

page_cache is a really bad decision on some of my sites. So suggesting that administrators install purge is a big problem for me. :D
Could this get merged?

krzysztof domański’s picture

krzysztof domański
Location Poland
commented
StatusFileSize
new purge-2803607-18.patch2.54 KB

re-rolled, it was unnecessary, the same as previous patch.

japerry’s picture

japerry
Location KOMK
commented
Issue tags: +purge-3.1

I also support removing this unneeded check.

  • nielsvm committed b6b6ba5 on 8.x-3.x authored by MiSc 
    Issue #2803607: Suggestion - remove PageCacheCheck, or document its...
nielsvm’s picture

nielsvm commented
Status: Reviewed & tested by the community » Fixed

Thanks everybody, it is gone now due unpopular demand :-) :

https://www.drupal.org/project/purge/issues/3236847

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

murat_halici’s picture

murat_halici commented

Removal of PageCacheCheck in 8.x-3.1 is causing a fatal error in all of our sites due to cached plugins.

Uncaught exception 'Drupal\Component\Plugin\Exception\PluginException' with message 'Plugin (page_cache) instance class "Drupal\purge\Plugin\Purge\DiagnosticCheck\PageCacheCheck" does not exist.' in /mnt/www/html/cbappcontent/docroot/core/lib/Drupal/Component/Plugin/Factory/DefaultFactory.php:97

Clearing caches resolves the issue. However, I was wondering if anyone knows of a way to prevent the error from occurring without having to do that.

ralphvdhoudt’s picture

ralphvdhoudt commented
StatusFileSize
new dummy-page_cache-plugin-to-prevent-errors-on-deploy-2803607.patch826 bytes

Added a dummy page_cache plugin file to be used as a temporary patch when updating/deploying your code

Fixes:
Uncaught exception 'Drupal\Component\Plugin\Exception\PluginException' with message 'Plugin (page_cache) instance class "Drupal\purge\Plugin\Purge\DiagnosticCheck\PageCacheCheck" does not exist.' in /mnt/www/html/cbappcontent/docroot/core/lib/Drupal/Component/Plugin/Factory/DefaultFactory.php:97

Patch is not intended to be included in the module itself