spatie / laravel-medialibrary

Associate files with Eloquent models
https://spatie.be/docs/laravel-medialibrary
MIT License
5.74k stars 1.07k forks source link

Conversions aren't always generated when queued and running parallel #3672

Closed royduin closed 2 months ago

royduin commented 2 months ago

We're having weird issues for a while where some conversions aren't generated. Hard te reproduce because it doesn't happen every time. On a model we do have 3 conversions; mini, thumb and detail. Mini isn't queued and is always generated. The other 2 are queued. In code:

public function registerMediaConversions(?Media $media = null): void
{
    $this->addMediaConversion('mini')
        ->fit(Fit::Crop, 100, 100)
        ->keepOriginalImageFormat()
        ->nonQueued();

    $this->addMediaConversion('thumb')
        ->fit(Fit::Crop, 360, 360)
        ->keepOriginalImageFormat();

    $this->addMediaConversion('detail')
        ->fit(Fit::Crop, 720, 720)
        ->keepOriginalImageFormat();
}

This in combination with Laravel Horizon and QUEUE_CONNECTION=redis and multiple workers:

'supervisor-1' => [
    'connection' => 'redis',
    'queue' => ['default'],
    'balance' => 'simple',
    'processes' => 5,
    'tries' => 3,
],

Does sometimes result in just {"mini": true} in the generated_conversions column instead of {"mini": true, "thumb": true, "detail": true}. The actual images aren't generated as well! Resulting in broken images in the frontend. In those cases the PerformConversionsJob job has a runtime of 0.00s in Horizon.

Running php artisan media-library:regenerate --only-missing does generate them but it should generate them directly.

I added some debugging and it seems like it's caused by a directory which is deleted which shouldn't happen. Related: https://github.com/spatie/laravel-medialibrary/pull/3630 but in another file; https://github.com/spatie/laravel-medialibrary/blob/bce761db9dc9d559365b256e98ad9bd5cef16192/src/Conversions/FileManipulator.php#L53 here the temp directory is created and the whole directory will be removed at the end: https://github.com/spatie/laravel-medialibrary/blob/bce761db9dc9d559365b256e98ad9bd5cef16192/src/Conversions/FileManipulator.php#L74

The temp directory should be unique: https://github.com/spatie/laravel-medialibrary/blob/bce761db9dc9d559365b256e98ad9bd5cef16192/src/Support/TemporaryDirectory.php#L19 but when I add some logging here: https://github.com/spatie/laravel-medialibrary/blob/bce761db9dc9d559365b256e98ad9bd5cef16192/src/Conversions/Actions/PerformManipulationsAction.php#L45 with logger($conversionTempFile); I'm getting:

/storage/media-library/temp/tzFZaU0bIYOVyG6fHdsSsmhnQujw6SxO/dz2O6rgEYt9cGXChnDt7GPtDIFiVZMDBdetail.jpg  
/storage/media-library/temp/P5vMHGcr60fGEBYUe5RHHKZ2vX7WepAT/zbunK1fPBOBLR5ffXRT1tyuxT3Hbv0Wkmini.jpg  
/storage/media-library/temp/4g76pGzlPIjm0eACMogyWsr93EXfdZlB/34cEGcRdwMs7l8jhx8JCc85Be7aUK8Gwmini.jpg  
/storage/media-library/temp/wNHNmdRzBYHHTyVOcEoS7o4j1SCwH9Hx/lbr8LQhqmCTbMI070RACkQfpJPWOXFn0mini.jpg  
/storage/media-library/temp/jKiPsdVzDtg5YOZqq9jTMI1nL6W6Kzye/rcfAnJN3UCF614qR8ZRqV1ozLQtkg3Epmini.jpg  
/storage/media-library/temp/rRsK4aWDoYP7ZEGgV1Or9JuFoS29m6hM/F6hlNEa9wHN43pAs1Alv0kj0bEGUHKv6thumb.jpg    
/storage/media-library/temp/kNCsZApUFbTIQTLlBnta7zVHdnkcVeEl/XjngrkVf98JpowpSfxYCgUV7s3z093wgthumb.jpg    
/storage/media-library/temp/rRsK4aWDoYP7ZEGgV1Or9JuFoS29m6hM/5MHSd1uc5fRuyTv20TdoCziygP0Z54jhdetail.jpg  
/storage/media-library/temp/kNCsZApUFbTIQTLlBnta7zVHdnkcVeEl/YD4x5nYMqxDmzvNtvu46X0GubvHei7mfdetail.jpg  

Where kNCsZApUFbTIQTLlBnta7zVHdnkcVeEl and rRsK4aWDoYP7ZEGgV1Or9JuFoS29m6hM are used twice.

So getTemporaryDirectoryPath() isn't generating unique directories... can we make that truly unique somehow? Or should be check if it's the only file in directory and only delete it in that case?

To reproduce it simply use the same directory here: https://github.com/spatie/laravel-medialibrary/blob/bce761db9dc9d559365b256e98ad9bd5cef16192/src/Support/TemporaryDirectory.php#L19 with for example:

return $path.DIRECTORY_SEPARATOR.'123';