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

Regression bug in Magento 2.1.6 - hidden images used on the product listing never get generated #9498

Closed
hostep opened this issue May 3, 2017 · 14 comments
Labels
bug report Component: Catalog Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@hostep
Copy link
Contributor

hostep commented May 3, 2017

This was originally reported by @airdrumz in #9276, but I think this problem deserves its own issue, otherwise it is going to get ignored.

Preconditions

  1. Magento 2.1.6
  2. PHP 7.0.18

Steps to reproduce

  1. Set up a brand new Magento 2.1.6 project
  2. Create a category
  3. Create a product, assign it to the new category, make sure you can see it on the frontend
  4. Assign an image to the product, and make it hidden by clicking it and checking 'Hide from Product Page' and save the product
  5. Go to the frontend and look at the product listing of the new category

Expected result

  1. Image on the product list is visible

Actual result

  1. A broken image on the product list

Discussion

This started happening after the decision to no longer generate resized images on the fly on the frontend in Magento 2.1.6. Images are now only being resized when a product is saved in the backend or by running the bin/magento catalog:images:resize command.
Both of these can't fix this problem, so you are stuck with this problem.

There is a way to work around it, and that is to unhide the image, save the product, then hide it again. Then the resized image will be generated. But you can't expect shopowners to do this manually for thousands of products?

I think this is a pretty urgent bug, and should be fixed in the next minor release.

@ghost
Copy link

ghost commented May 8, 2017

For me bin/magento catalog:images:resize - required almost 3 hours of time frame. At the time of final migration before launching - how could we wait for 3 hours? This issue must be resolved immediately

@hostep
Copy link
Contributor Author

hostep commented May 8, 2017

@ravibhalodia: you are talking about #9275 or #9276, this ticket isn't about that.
(but I agree with the 3 hours problem, you can try out this super experimental attempt at solving that: https://github.com/baldwin-agency/magento2-module-magento-216-image-resize-migration-helper)

@erfanimani
Copy link
Contributor

Pretty urgent yeah - any hack we can do make it work for now?

@hostep
Copy link
Contributor Author

hostep commented May 9, 2017

I posted a workaround in the original thread, let me copy it here, since it makes more sense to have it here:

diff --git a/app/code/Magento/Catalog/Model/Product.php b/app/code/Magento/Catalog/Model/Product.php
index 6cfa90a7c4c..54872986fe2 100644
--- a/app/code/Magento/Catalog/Model/Product.php
+++ b/app/code/Magento/Catalog/Model/Product.php
@@ -1461,13 +1461,13 @@ class Product extends \Magento\Catalog\Model\AbstractModel implements
      *
      * @return \Magento\Framework\Data\Collection
      */
-    public function getMediaGalleryImages()
+    public function getMediaGalleryImages($includeDisabledImages = false)
     {
         $directory = $this->_filesystem->getDirectoryRead(DirectoryList::MEDIA);
         if (!$this->hasData('media_gallery_images') && is_array($this->getMediaGallery('images'))) {
             $images = $this->_collectionFactory->create();
             foreach ($this->getMediaGallery('images') as $image) {
-                if ((isset($image['disabled']) && $image['disabled']) || empty($image['value_id'])) {
+                if ((isset($image['disabled']) && $image['disabled'] && !$includeDisabledImages) || empty($image['value_id'])) {
                     continue;
                 }
                 $image['url'] = $this->getMediaConfig()->getMediaUrl($image['file']);
diff --git a/app/code/Magento/Catalog/Model/Product/Image/Cache.php b/app/code/Magento/Catalog/Model/Product/Image/Cache.php
index 2a5316583ff..4f0bbbfa0e9 100644
--- a/app/code/Magento/Catalog/Model/Product/Image/Cache.php
+++ b/app/code/Magento/Catalog/Model/Product/Image/Cache.php
@@ -81,7 +81,7 @@ class Cache
      */
     public function generate(Product $product)
     {
-        $galleryImages = $product->getMediaGalleryImages();
+        $galleryImages = $product->getMediaGalleryImages(true);
         if ($galleryImages) {
             foreach ($galleryImages as $image) {
                 foreach ($this->getData() as $imageData) {

Disclaimer: not tested thoroughly!

@erfanimani
Copy link
Contributor

erfanimani commented May 9, 2017 via email

@tbrankaer
Copy link

I'm having the same issue in 2.1.6. Any news about this yet? Does this fix work?
Thanks!

@burgh8wp
Copy link

Experiencing this same problem. Upgraded from 2.1.5 to 2.1.6 and when we click the flush images button in System > Cache Management the pub/media/catalog/product/cache folder is deleted and not re-generated. Thus images disappear. The bin/magento catalog:images:resize command fixes the issue. Although takes an age to run on our 4000 products in this particular store! So by no means a solution. Proper fix in next version of Magento2 would be great.

@danemacmillan
Copy link

danemacmillan commented May 25, 2017

@hostep
This started happening after the decision to no longer generate resized images on the fly on the frontend in Magento 2.1.6.

Was this an actual decision by the Magento2 team? (Edit: Yes, it's in the updated changelog.) It was deliberate? Is this permanent? On-the-fly image generation is the only method that will scale for any large store. My store has 381,185 SKUs; it has 238,050 catalog images, totalling 41GB. On Magento1, when including cached images, that size jumps to 148GB. It would have been great if the cache URL remained the same as on Magento1, but it doesn't. Instead, they are all new URLs, and for some ungodly reason, about 37 thumbnails are generated per image. My store, then, is supposed to run that command, pray it finishes, and result in 238,050*37=8,807,850 images? Even our 148GB is only 2,749,525 files. Where am I realistically expected to store all these thumbnails?

We can't seriously be expected to run that fickle command (bin/magento catalog:images:resize) that fails the moment a problem file is thrown at it, like a zero-length file? Not to mention, it is not intelligent enough to skip files that have already been generated, so when an error happens (and no information provided, of course), the whole process needs to be started over, and the thumbnails that were successfully generated are just regenerated and overwritten again. I've yet to successfully complete the operation. It typically dies around the ten hour mark, where, I'm assuming, it fails on some image, though I have no idea which one it could be, because Magento.

Edit: The latest run died about an hour and a half in. How is anyone expected to resolve a cryptic, "Unsupported image format," message? How is this useful? How is anyone expected to resolve this? Major work needs to be done.

screen shot 2017-05-25 at 7 37 56 pm

@iangoh414
Copy link

Hi,

we are facing the same problem.
We have 5000 items and this is definitely not a solution! I cannot believe how the magento team decided on this implementation.

image

@dersam
Copy link

dersam commented May 29, 2017

I'm working on the same store as @danemacmillan (381,185 SKUs). I modified the catalog:image:resize command to catch any errors related to image processing and ignore them, simply to allow it to complete. I don't consider that a solution, just part of the diagnostic process.

This modification revealed another issue. The catalog:image:resize command leaks memory like a sieve. The process crashed with an out of memory error after processing 25852 products (about 7% of our total catalog). Total memory in use at the time was 998MB (max memory for PHP on the test machine). This climbed at a constant rate from the initial 120MB at the start of the process, showing no signs of levelling off. Every 25-30 products processed added about another 1MB to memory usage.

This appears to be related to the \Magento\Catalog\Model\ProductRepository storing any loaded product in a local property. As more products are examined for media galleries, the memory in use climbs, and is never disposed. This doesn't appear to be the only class where this is the case, either. A quick hack to ensure the cache of the ProductRepository wasn't populated revealed that memory usage still climbs, but at a lower rate. That indicates that there are further memory leaks in the process, most likely related to similar in-memory caching.

The catalog:image:resize command simply isn't adequate for handling genuinely large catalogs. A 10 hour processing time is absurd, especially when the process does not gracefully handle errors. On-the-fly image generation needs to be restored.

@mzeis
Copy link
Contributor

mzeis commented May 31, 2017

Magento just released 2.1.7 with a "reversion of the changes to image resizing that we introduced in 2.1.6": http://devdocs.magento.com/guides/v2.1/release-notes/ReleaseNotes2.1.7CE.html

Can you please check if the original problem (not the performance issue, I guess) is gone in 2.1.7? Thanks!

@KrystynaKabannyk
Copy link

hi @hostep, thank you for reporting this issue, ticket MAGETWO-69570 is created for it.

@KrystynaKabannyk KrystynaKabannyk added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Component: Catalog labels Jun 1, 2017
@KrystynaKabannyk
Copy link

Verified that changes to image resizing that have been introduced in 2.1.6 fixed the issue, that's why I'm closing the issue.

@hostep
Copy link
Contributor Author

hostep commented Jun 8, 2017

@KrystynaKabannyk: Why are you closing this issue? I can't see any commits referring MAGETWO-69570?

I know it has been fixed in Magento 2.1.7 by reverting the new image resizing logic, but this doesn't mean the bug is fixed on the develop branch? Or are you reverting the image resizing logic over there as well?

I also don't understand your comment, you are saying 2.1.6 fixed the issue but that's completely incorrect, because the bug was actually introduced in 2.1.6...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: Catalog Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests

9 participants