Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed module Mage_PageCache #2258

Merged
merged 6 commits into from
Jul 15, 2022

Conversation

fballiano
Copy link
Contributor

@fballiano fballiano commented Jun 26, 2022

The Mage_PageCache was a half baked first attempt at pagecache for community edition.
As far as I can see it lacks basic features like holepunching and support for modern technologies like redis.
It only supports Zend_Server which probably doesn't even exist anymore.

PS: I also found out that, just having it enabled, broke my varnish/turpentine.

@github-actions github-actions bot added Component: Adminhtml Relates to Mage_Adminhtml Component: PageCache Relates to Mage_PageCache Template : admin Relates to admin template Template : base Relates to base template translations Relates to app/locale XML Layout labels Jun 26, 2022
@ADDISON74
Copy link
Collaborator

ADDISON74 commented Jun 26, 2022

This module is related to "External Full Page Cache Settings" section in Backend. Even you Enable the module it works only if there is an external cache. See the image bellow:

page_cache

The only "External Cache Control" option available is "Zend Full Page Cache". Then you are able to flush the external page cache from "Cache Storage Management" page:

external page cache

How many use Zend server? I never use it and I consider it far inferior to the Varnish-based Turpentine extension. If anyone is using it please let us know. If there is no feedback in 2 weeks I will approve it on my behalf.

@ADDISON74
Copy link
Collaborator

You have to mention the removal of this module from OpenMage in the README.md file. There is a section named "Removed Modules".

@fballiano
Copy link
Contributor Author

done

@ADDISON74
Copy link
Collaborator

From what I notice we have no opinions to keep this module in OpenMage. Therefore I don't have to wait two weeks to approve the PR. I clicked the button.

There is a conflict related to an XML that must be resolved.

ADDISON74
ADDISON74 previously approved these changes Jun 30, 2022
@fballiano
Copy link
Contributor Author

fixed the conflict and a typo ;-)

ADDISON74
ADDISON74 previously approved these changes Jun 30, 2022
tmewes
tmewes previously approved these changes Jun 30, 2022
@Flyingmana
Copy link
Member

would need to check, but I think a php based fullPage cache I build 7y ago could have used this as base.

but guess removing it ks ok, if no modern module uses parts of it.

@fballiano
Copy link
Contributor Author

I'd just suggest to use Lesti :-)

@Flyingmana
Copy link
Member

I'd just suggest to use Lesti :-)

mine might have been less flexible, but I got constantly ~40-60ms response times on cached pages. Is it comparable in performance?

@fballiano
Copy link
Contributor Author

No they're much higher (at least on the production website I'm using it in) but it's probably because of the holepunching dynamic blocks

@fballiano fballiano dismissed stale reviews from tmewes and ADDISON74 via ce435d1 July 1, 2022 09:48
@fballiano
Copy link
Contributor Author

fixed phpstan

@fballiano fballiano requested a review from ADDISON74 July 13, 2022 12:01
ADDISON74
ADDISON74 previously approved these changes Jul 13, 2022
@fballiano
Copy link
Contributor Author

fixed conflict in README

Copy link
Member

@colinmollenhour colinmollenhour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this page cache when I reviewed it appeared to be almost unusable.

@fballiano fballiano merged commit 84b65c3 into OpenMage:20.0 Jul 15, 2022
@fballiano fballiano deleted the remove_pagecache_module branch July 15, 2022 19:13
@github-actions
Copy link

Unit Test Results

0 files  ±0  0 suites  ±0   0s ⏱️ ±0s
0 tests ±0  0 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 84b65c3. ± Comparison against base commit d91170b.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Adminhtml Relates to Mage_Adminhtml Component: PageCache Relates to Mage_PageCache documentation environment Template : admin Relates to admin template Template : base Relates to base template translations Relates to app/locale XML Layout
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants