kamsar / Dianoga

An automatic image optimizer for the Sitecore media library.
Other
104 stars 45 forks source link

Dianoga on production CD is weird with resizing #70

Closed izharikov closed 3 years ago

izharikov commented 4 years ago

Dianoga version

Dianoga 5.0.1

Sitecore version

Sitecore 9.0.1

Config

Tried with multiple configurations: Sync, async modes, WebP enabled/disabled Latest config: Screenshot_17

Steps to reproduce & results

No actual steps to reproduce. What we did:

  1. Upgraded Dianoga from 3 to 5
  2. So, we deleted Dianoga config folder & emptied the Media Cache
  3. Started PROD CD server

Expected: Dianoga works well with resizing.

Actual:

  1. With WebP enabled many images are not resized correctly: e.g. they need to be resized 300x600px, but actually they are 300x300px: in both sync & async mode
  2. With WebP disabled (in Async mode) looks like many images are mixed inside MediaCache: another media item is returned, when resize using (with wrong dimensions).

P.S.

I clear realize, that this description is very abstract and there's no actual steps to reproduce. By the way I have some ideas:

  1. Something is configured not correctly
  2. There're issues with multi-threading (cause on non-production environments everything looks good)
  3. There're issues inside dependent tools

Maybe someone else faced with issue like this? Cause, Dianoga is great tool, but now we disabled it because of wrong images generated.

markgibbons25 commented 4 years ago

Is it possible that after disabling WebP, the media cache wasn't fully cleared, so it was still serving some WebP images?

izharikov commented 4 years ago

@markgibbons25, no I checked in chrome dev tools, that images are served in png and jpg formats.

markgibbons25 commented 4 years ago

WebP enabled many images are not resized correctly: e.g. they need to be resized 300x600px, but actually they are 300x300px

Can you give more detail on how you are rendering the link URL for this example, and what the final url query string is rendering as.

izharikov commented 4 years ago

Final URL looked like [host]/de-at/-/media/path-to-media-item?rev=4a49853f3b2e4116b41f19b0e02a7eda&h=320&w=640&la=de&hash=ADDA85DDA60C921C816016933AB8817BFD29F0EE'

Where the original item was '/de-at/-/media/path-to-media-item' e.g. 1280x640.

The strangest thing was that after clearing the cached image for this exact medium to restart the regeneration, the image was correctly compressed and resized. But then the problem could be found elsewhere.

So, approximately 5-10% of images didn't resized & compressed correctly, but it was too much for us.

ozkansayin commented 3 years ago

Found the issue.

WebPOptimizer customizes AdditionalToolArguments property whenever an image is being processed. However, the value of that property is shared among concurrent optimizations. Hence, it was being overwritten by concurrent runs, messing with the -resize parameter for the tool.

PR to fix it created: https://github.com/kamsar/Dianoga/pull/81

markgibbons25 commented 3 years ago

Thanks @ozkansayin great work! Added to 5.2.0

izharikov commented 3 years ago

@ozkansayin, that's kind of crazy, that you found and solved this issue. Great job, thanks!