Skip to content

#6942 added check for page layout loaded from cache #16428

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

#6942 added check for page layout loaded from cache #16428

wants to merge 2 commits into from

Conversation

mrIntegrator
Copy link

@mrIntegrator mrIntegrator commented Jun 27, 2018 •
edited
Loading

This is the 2.2-develop version of #14129. Description copied below.

Description

Under heavy traffic, some pages will become broken because the HTML <head> is not present. The only remedy is to disable layout cache.
When layout updates are loaded from cache, it's assumed that page layout will be loaded from cache as well. The problem is that when Redis reaches its maxmemory setting (as it might under heavy traffic), it will begin evicting keys. So if the page layout key is evicted while the layout key is not, the bug occurs. To fix this bug, we have to check for a successful load of both layout and page layout from the cache.

Fixed Issues (if relevant)

  1. Layout corruption with php-fpm (a Magento 1 problem returns) #6942 (Title is wrong, but issue is real)

Manual testing scenarios

  1. Install Magento with Redis cache (I used db 0 for sessions, 1 for cache, 2 for FPC).
  2. Navigate to a page not cached by FPC (I used customer/account/login).
  3. Delete the page layout cache key in Redis. (Found by doing a var_dump of $cacheIdPageLayout in Magento\Framework\View\Model\Layout\Merge::load).
  4. Refresh the page and behold the headlessness of your broken page.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-engcom-team
Copy link
Contributor

Hi @mrIntegrator. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me {$VERSION} instance - deploy vanilla Magento instance

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

@mrIntegrator mrIntegrator mentioned this pull request Jun 27, 2018
Closed
4 tasks
@orlangur orlangur self-assigned this Jun 27, 2018
@magento-engcom-team magento-engcom-team added this to the Release: 2.2.6 milestone Jul 10, 2018
@magento-engcom-team
Copy link
Contributor

Hi @orlangur, thank you for the review.
ENGCOM-2264 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

@mrIntegrator thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

@ihor-sviziev ihor-sviziev mentioned this pull request Aug 23, 2018
Closed
@orlangur
Copy link
Contributor

orlangur commented Nov 7, 2018

Just noticed a comment regarding performance degradation, will check if slightly changing implementation helps.

@okorshenko okorshenko removed this from the Release: 2.2.8 milestone Jan 28, 2019
@sivaschenko
Copy link
Member

Hi @mrIntegrator @orlangur , any updates on this pull request?

@sidolov
Copy link
Contributor

sidolov commented Feb 26, 2019

@mrIntegrator , I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for the collaboration!

@sidolov sidolov closed this Feb 26, 2019
@ghost
Copy link

ghost commented Feb 26, 2019

Hi @mrIntegrator, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@Swahjak Swahjak mentioned this pull request Mar 25, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@orlangur orlangur orlangur approved these changes

Assignees

@orlangur orlangur

@sivaschenko sivaschenko

Labels
Component: Framework/View help wanted Progress: accept Progress: needs update Release Line: 2.2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mrIntegrator @magento-engcom-team @orlangur @sivaschenko @sidolov @okorshenko