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

FPC for unrelated products being cleared after editing a single product #25670

Closed
gibbotronic opened this issue Nov 20, 2019 · 21 comments
Closed
Assignees
Labels
Component: PageCache Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Progress: done Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@gibbotronic
Copy link

gibbotronic commented Nov 20, 2019

Preconditions (*)

  1. Magento 2.3.3 & 2.3-develop

Steps to reproduce (*)

  1. Load the demo store

  2. Ensure that demo store is set to use FPC (Store > Config > System > Cahce > Full Page Cache

  3. Load some product pages on the front end eg in Chrome. (Use the network tab in the browser inspector to note load time - TTFB should be ~700+ms ish)

  4. Reload all pages. TTFB should be reduced to ~150ms ish as the pages have been cached.
    image

  5. Edit an unrelated product on the back of the site (not present in any related products etc).
    e.g.You can move product from first category to second

  6. Reload all front end pages one at a time. Note that the TTFB has now gone back up to ~700+ms indicating a clearing of cache, even though the only modified item was an unrelated product.
    Note: This also applies to home page, categories etc.
    image

Expected result (*)

  1. The FPC should not be cleared for unrelated products.

Actual result (*)

  1. The cache is getting cleared as indicated by the increase in TTFB.
@m2-assistant
Copy link

m2-assistant bot commented Nov 20, 2019

Hi @gibbotronic. Thank you for your report.
To help us process this issue please make sure that you provided the following information:

  • Summary of the issue
  • Information on your environment
  • Steps to reproduce
  • Expected and actual results

Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:

@magento give me 2.3-develop instance - upcoming 2.3.x release

For more details, please, review the Magento Contributor Assistant documentation.

@gibbotronic do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

  • yes
  • no

@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Nov 20, 2019
@gibbotronic
Copy link
Author

@magento give me 2.3-develop instance

@magento-engcom-team
Copy link
Contributor

Hi @gibbotronic. Thank you for your request. I'm working on Magento 2.3-develop instance for you

@magento-engcom-team
Copy link
Contributor

Hi @gibbotronic, here is your Magento instance.
Admin access: https://i-25670-2-3-develop.instances.magento-community.engineering/admin
Login: admin Password: 123123q
Instance will be terminated in up to 3 hours.

@gibbotronic
Copy link
Author

There is no data on that instance. I have tried adding 2 products and editing one but there are no indexers running on that instance and I have no way of forcing the indexers to run, so it is not a suitable test scenario for the bug.

Due to the this I cannot fully confirm on that instance, so I am marking this bug as “reproducible” on vanilla 2.3.3.

@gibbotronic do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?

yes <---------- YES

@m2-assistant
Copy link

m2-assistant bot commented Nov 21, 2019

Hi @engcom-Charlie. Thank you for working on this issue.
In order to make sure that issue has enough information and ready for development, please read and check the following instruction: 👇

  • 1. Verify that issue has all the required information. (Preconditions, Steps to reproduce, Expected result, Actual result).

    DetailsIf the issue has a valid description, the label Issue: Format is valid will be added to the issue automatically. Please, edit issue description if needed, until label Issue: Format is valid appears.

  • 2. Verify that issue has a meaningful description and provides enough information to reproduce the issue. If the report is valid, add Issue: Clear Description label to the issue by yourself.

  • 3. Add Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

  • 4. Verify that the issue is reproducible on 2.3-develop branch

    Details- Add the comment @magento give me 2.3-develop instance to deploy test instance on Magento infrastructure.
    - If the issue is reproducible on 2.3-develop branch, please, add the label Reproduced on 2.3.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

  • 5. Add label Issue: Confirmed once verification is complete.

  • 6. Make sure that automatic system confirms that report has been added to the backlog.

@engcom-Charlie engcom-Charlie added Component: PageCache Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Nov 22, 2019
@ghost ghost unassigned engcom-Charlie Nov 22, 2019
@magento-engcom-team
Copy link
Contributor

✅ Confirmed by @engcom-Charlie
Thank you for verifying the issue. Based on the provided information internal tickets MC-29069 were created

Issue Available: @engcom-Charlie, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself.

@magento-engcom-team magento-engcom-team added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Nov 22, 2019
@gibbotronic
Copy link
Author

gibbotronic commented Jan 20, 2020

Unfortunately, the FPC in Magento is broken by design and cannot be fixed.

On lengthy and tedious investigation, the issue stems from some of the logic being in the UI templates (XML + .phtml or any of the myriad languages that extensibility can be written in, with varying locations of interpretation/compilation/callback), and some of it being in the program code, and the attributes being siblings without any annotation/flag of their importance.

Due to the "extend everything" design of Magento with its "beforeSave/afterSave" event hooks littered for every single object type, there is no way to deterministically know what has been changed/invalidated by modifying a single attribute under a product. Therefore Magento takes the blanket "invalidate everything" approach.

Here's an explanation:
a. When updating a product, the system knows that something in the product has changed. What has changed may be visible on the product page, or it may not be. It doesn't know, because the decision whether to display something on the product page will be dictated by:
i. the layout XML blocks (with potential conditional behaviour in there driven by the block attributes), and/or
ii. the .phtml that outputs the blocks (with potential conditional behaviour in there driven by any variables it wishes to load from Magento), and/or
iii. the extra x-magento-init etc. that is ran AFTER the .phtml has been ran (or whenever, who knows!) (with potential conditional behaviour in there driven by whatever it wishes to load/phase of the Moon etc.).
As all attributes under a product are SIBLINGS (they have no hierarchy) and do not have any tags or properties to indicate their importance or whether they are visible on a user-facing page at all (remember - the logic for that is dictated by the i, ii, iii mentioned above), Magento doesn't know that you modifying a backend attribute will not actually make any difference whatsoever to the page cache that the user will see.
This is also true for products if they sit under categories - will modifying an attribute be visible or have any effect on the category that the product belongs to? It doesn't (and CANNOT) know.

b. It will flush the cache for itself, and its parent if it is configurable product.

c. It will find all of the related categories to itself, so the parent category that the product sits in. It will flush that too.
Remember, it cannot know if the attribute that you modified will be visible on any category page that the product belongs to.

d. It will also flush the parent of that category, and the parent of that category, recursively until the root category is reached, thereby flushing the ENTIRE system.
Remember, it doesn't know that modifying a product within a category will affect the parent of that category, or the parent of that category, or the parent of THAT category, and so on and so on forever.

It just knows that something in the product has changed! It has no concept of its importance or whether it is user-visible or anything.

If a product is related to another product via a crosssell or upsell, then will modifying this product also affect the layout of the RELATED products? Again, it doesn't know. The logic dictating whether an item is shown or not is not interpretable at the point of save. It'll only be interpreted when the related events fire for displaying the other product (building the other product's blocks).

The same is true for modifying categories. It knows that something in the category has changed, so goes and invalidates the entire universe just in case it affects it. It can't risk NOT invalidating it because the change might need to be visible.

A potential solution would be to annotate attributes as being user-facing with a boolean flag, but this is fallible and not provable by code since a user could not bother marking something as user-facing but then go and refer to it or use it in any of the .phtml / other scripts used for laying out pages, unless you catastrophically fail when they use such an attribute illegally but that'll cause a widespread panic.
The real solution would be to ditch the layout logic mechanism being in the .phtml and magento script and force it to be expressed via XML alone, which could be parsed to inspect for changes and deterministically know what will be affected by a given change but this is never ever going to happen given the colossal market for templates + modules that exists. It's a horrible mess.

One issue I have found is that Magento stupidly kicks off a "flush the entire Varnish cache" request under app/code/Magento/PageCache/Model/Cache/Type.php clean function, where it ignores the incoming $mode and makes a "adminhtml_cache_refresh_type" request which ends up in app/code/Magento/CacheInvalidate/Observer/FlushAllCacheObserver.php execute function, thereby destroying the entire Varnish cache.
This is probably affecting everyone using Varnish + Magento and contributing to millions of CPU cycles, using more power, thereby burning more fossil fuels and contributing to the eventual death of Earth.
bugScreenshot

You can test the assertions within this comment by asking yourself: "if I modify an attribute of a product, what else will be affected or invalidated?"
If you cannot reliably define what has changed or needs to be updated (you won't be able to), then you must assume that everything that somehow is related to the modified product has changed.
Like you, Magento also does not know due to the scattering of logic into so many places.

@TimQSO
Copy link

TimQSO commented Jan 20, 2020

I'm debugging this for two years now and my findings are the same as yours. With the big difference that you were able to write it out and spend your time on that. I appreciate that.

Unfortunately. Adobe is busy adding stuff to Magento no one needs while forgetting there are 1000+ open bugs and one of the most important bugs that isn't working in Magento: FPC.

My servers are indeed contributing to millions of CPU cycles and the death of the Earth, because my cache warmer is warming up 1.3 million pages a day, trying to keep up the with cache flushes. I'm lucky that I've very fast and bad ass servers (multi Xeon - 40 cores 128GB RAM)

Because, that is what happens if you have a busy Magento ecommerce shop....

It is saddening that this is the case. I really don't have faith someone within Magento even bothers to look at this at all. Because dozens of people are filing bug reports and they are being closed as not reproducable or they are eventually being closed because of inactivity.

While this is something the core Magento team should fix as top priority instead of wasting time making paid modules no one wants to use.

@gibbotronic
Copy link
Author

gibbotronic commented Jan 20, 2020

That's sad and a lot of pages to be updated! I am not sure how it can be fixed TBH as the logic is in too many places and in too many languages, some of which is interpreted by callbacks after the page is loaded.

Instead:

  1. The logic for products (and anything else, down to an attribute granularity) needs to be expressed in a single, parseable place that is interrogate-able at the point of save. This is NOT how Magento works, as it works by constructing the blocks as they are requested, which is only done after the object is marked as dirty/changed.
  2. It needs to be able to know if anything uses a given attribute. Again, this is NOT how Magento works as multiple, non-queryable languages have been introduced for writing extensions and modules, and they are interpreted at different stages of the block building (eg. compilation of themes, building of cached files, callback of client-side javascript).

I am sad to say that the entire system needs a rewrite for it to ever support FPC. I cannot possibly see how any component can deterministically answer the question "do you use this attribute?" in its current form, particularly as the event system is dynamic and therefore not statically analysable, ever.

BTW, this extensible modular design leads to prolific non-optimised database loads because no component can ever know if another component needs the same info; eg. a fresh out-of-the-box installation will run 86 SQL queries just to display a product page, which is baffling overkill.

@sdouma
Copy link

sdouma commented Feb 13, 2020

seriously magento, why hasn't this gained more traction

@gibbotronic
Copy link
Author

Is there any comment on this at all? It's been months since I reported it, and over a month since my last comment detailing my findings, where I do not see it being "fixable" in the current design.

Comments????

@TimQSO
Copy link

TimQSO commented Mar 2, 2020

It has something to do with the categories that are added to the Top Menu (that is the main menu bar of your shop).

When you disable all categories for the top menu as a test (You wont see any menu items any more in your shop) and then save a product, you will see that the cache will only be invalidated for that product and not the other products. As soon as you enable the categories for the top menu again, the issue will come back.

With the top menu enabled the products get a general cat_c tag. And saving one product will cause a flush for the cat_c tag and that means everything. When you disable the top menu categories, the cat_c tag will not be added to those products anymore.

There are so many tickets about caching issues in Magento, but nobody within Magento seems to want to take a look at it.

@lenaorobei
Copy link
Contributor

This issue will be addressed in the scope of internal ticket MC-32337.

@m2-assistant
Copy link

m2-assistant bot commented Apr 14, 2020

Hi @vzabaznov. Thank you for working on this issue.
Looks like this issue is already verified and confirmed. But if you want to validate it one more time, please, go though the following instruction:

  • 1. Add/Edit Component: XXXXX label(s) to the ticket, indicating the components it may be related to.

  • 2. Verify that the issue is reproducible on 2.4-develop branch

    Details- Add the comment @magento give me 2.4-develop instance to deploy test instance on Magento infrastructure.
    - If the issue is reproducible on 2.4-develop branch, please, add the label Reproduced on 2.4.x.
    - If the issue is not reproducible, add your comment that issue is not reproducible and close the issue and stop verification process here!

  • 3. If the issue is not relevant or is not reproducible any more, feel free to close it.


@gibbotronic
Copy link
Author

Thanks for an update. What does this internal ticket involve? I cannot log in to see it.

What will the solution involve? Each one I can think of has drawbacks:

  1. Annotate attributes as "user visible" (and only mark the product as dirty if one of these attributes is modified). This would need differentiating between "visible on category pages") and "visible on product pages". Fail if a non user-visible attribute is used in a template. It would be a bit of a hack because it still can't answer the question "what uses this attribute?".
  2. Parse the theme files to build a list of attributes used and mark the product as dirty (cache rebuild) when these attributes are changed against a file. (You might as well build the cache at this point though, and all of the logic in extensions/modules might use these attributes but you'd never detect their use, so you'd miss user-facing attributes).
  3. Force .phtml and PHP and XML template usage only (things you can parse server-side) and abandon x-magento-script callbacks. Since the script callbacks run after all of the other code, you absolutely no idea what they're using or the attributes they're using and you can't interrogate them or ask them. You can't even know if they're using other callbacks so have zero visibility of what gets ran (what a mess). Abandoning the script callbacks would mean containing all logic to the server, where you have greater chance of detecting what is being used, and when (but this still has the issue of only detecting attribute use after their requested...).
  4. Invent a new strict flat template/theme system (XML) that lists the attributes it is using and do not dynamically load blocks (which might use attributes you can't detect without traversing the block and its children etc.). Do not permit code to be ran in the theme, other than "load" or "save" so you can detect what would be read /written before ever rendering the theme. This would make the theme parseable and you would know what it uses without having to render it.
    You would know at the point of save of a product which attributes are user-visible. This would solve the problem but it's never going to happen because it'll destroy the existing theme market.

So I would be interested in the proposed solution? How can I view this internal ticket?

@TimQSO
Copy link

TimQSO commented Apr 15, 2020

Don't get your hopes up. I've seen several issues that were assigned to an internal ticket and still are waiting to be developed after 3 years...

@gibbotronic
Copy link
Author

Don't get your hopes up. I've seen several issues that were assigned to an internal ticket and still are waiting to be developed after 3 years...

That's a shame! How could you track their development if they're internal?

@TimQSO
Copy link

TimQSO commented Apr 15, 2020

You can't. At least not that I know.

@AntonEvers
Copy link
Contributor

Hi all, a PR related to this ticket was merged here: f442fa9. The internal ticket was closed after merging this PR.

@sdzhepa
Copy link
Contributor

sdzhepa commented Sep 16, 2020

All internal PRs with fixes had been delivered #25670 (comment)

We can close this issue as fixed.
But if some part was not fixed or missed, please create new Issue with reference to this one

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: PageCache Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Progress: done Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

9 participants