spatie / laravel-medialibrary

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

Flood on medialibrary:regenerate #1538

Closed francoism90 closed 4 years ago

francoism90 commented 5 years ago

When using the following command to regenerate the following 7 models:

$ artisan medialibrary:regenerate --only=thumbnail --ids=1,2,3,4,5,6,7

And something goes wrong, e..g.:

[mov,mp4,m4a,3gp,3g2,mj2 @ 0x5624f5b560c0] moov atom not found
/tmp/medialibrary/folder/file.mp4: Invalid data found when processing input

This happens:

$ ls /tmp/medialibrary
drwxr-xr-x 2 http http 100 Aug 24 18:09 1XuW3qr1kTdGOnEoT32nfQFX65LskSyc
drwxr-xr-x 2 http http  60 Aug 24 18:05 22npuAaPPlcD46qXtmzOyb8eAJlqqRX1
drwxr-xr-x 2 http http  60 Aug 24 18:05 6dw6fuIYwR7Wi4hR8gfsRHZazBKDSo2U
drwxr-xr-x 2 http http  60 Aug 24 18:05 8J4p1NwHBIsSG5u9sDhx9I6XXT5UQCFg
drwxr-xr-x 2 http http  60 Aug 24 18:09 9kjYYiPQz0uHbeCaMHRSUgamEOZH6le8
drwxr-xr-x 2 http http  60 Aug 24 18:06 9MlVYu396q44PVuICvdPGcjap6Bppu75
drwxr-xr-x 2 http http  60 Aug 24 18:09 CuwkZHJKGFpa189UGFu5R3I0SXliBhT0
drwxr-xr-x 2 http http  60 Aug 24 18:09 Dg8YrIrHnStKvyuNiHJ4WaDqflncJrkh
drwxr-xr-x 2 http http  60 Aug 24 18:09 E2ZECaDw0ndsdb5APYHkhqxGoIfCEOD7
drwxr-xr-x 2 http http  60 Aug 24 18:08 EfdvQNpMLC44HANNMvjL2xaMycJGr478
drwxr-xr-x 2 http http  60 Aug 24 18:06 eoIvBUME5xmvLVfjiuE8tMqXWkkPEDpF
drwxr-xr-x 2 http http  60 Aug 24 18:07 EvEhy5yOVgkCbBCAas1JhCf8BItbZyEZ
drwxr-xr-x 2 http http  60 Aug 24 18:09 gKKuKYjoLEcDQJXnHqDNDR2ZVM9cK83w
drwxr-xr-x 2 http http  60 Aug 24 18:06 hwHKmt2R78hXrL89HBvM6BKUwd9epqCt
drwxr-xr-x 2 http http  60 Aug 24 18:05 imwEgxYE7a7oumrPSWCJ4Mj0ouwehcci
drwxr-xr-x 2 http http 100 Aug 24 18:07 L3e2NWoilAvsu2QNFcOcLImTyAc20sUp
drwxr-xr-x 2 http http  60 Aug 24 18:05 laZ9ZKfw8M2vJfUHlY25oXo2huiQF71z
drwxr-xr-x 2 http http  60 Aug 24 18:05 lR860MAXLMizgCD7kQYkDIyHmOYKLVF5
drwxr-xr-x 2 http http  60 Aug 24 18:05 nP9vdrrSdMg95joiBw7PCPj5a8qeGS0x
drwxr-xr-x 2 http http  60 Aug 24 18:09 nuLlnP1VvQ66wSS2rS622VkzGpontsyp
drwxr-xr-x 2 http http 100 Aug 24 18:09 oag2FFsWqBoXiR1aSoM8uix3RFbNYJQs
drwxr-xr-x 2 http http  60 Aug 24 18:09 oJ5XUCbtfaSu8HCGxeqWKuvS7BXRNoJB
drwxr-xr-x 2 http http  60 Aug 24 18:05 q7KfhECyIL1ixsBUF4r6jrJO4bi1Xdsf
drwxr-xr-x 2 http http 100 Aug 24 18:07 QhpxmjnaToOkgS6fR8nZ7Lc9tIKL3X9B
drwxr-xr-x 2 http http  60 Aug 24 18:07 rqDKEDJtdTyKxbnb7RgNHr3EkbWTvw1t
drwxr-xr-x 2 http http  60 Aug 24 18:07 sfIBw5doQaX2b2yRbadsNnqdNxGNwUwO
drwxr-xr-x 2 http http  60 Aug 24 18:07 viPoKlVZQwpfTXck0HIsJobiDkqMUkgN
drwxr-xr-x 2 http http 100 Aug 24 18:09 WJySfDowxXU49jvCIjFqbKjZv7C8GJik
drwxr-xr-x 2 http http  60 Aug 24 18:07 y2rz88xCw2Q5NTaMFyojVlPukCy34ZZC

It seems to duplicate like crazy and it's filling up the drive space. I've tried multiple locations (even non cache), but I'm unable to address this issue.

Do you have an idea?

Thanks.

brendt commented 5 years ago

Are you using a sync queue driver or horizon? You should use the latter one.

francoism90 commented 5 years ago

@brendt I'm using Horizon as queue driver and using auto as preset.

I found out that multiple jobs of the same Media-model are spawn when it has an invalid character in the filename and FFMpeg cannot probe the file, resulting in a loop. When the invalid char(s) have been fixed/removed the regenerate progress runs fine and the loop doesn't occur.

Is it true the PerformConversions job doesn't have any throttle? It would be great to lower the number of max. allowed conversions that happen in a specific time period, preventing the jobs from stalling. Or is this possible already? :)

brendt commented 5 years ago

We probably could make improvements in this area. But we need to take into account that Laravel's throttling only works with Redis (https://laravel.com/docs/5.8/queues#rate-limiting), I don't think it'll be straight forward to add.

I found out that multiple jobs of the same Media-model are spawn when it has an invalid character in the filename and FFMpeg cannot probe the file, resulting in a loop.

Are you able to constantly reproduce this? Are you willing to submit a PR with a failing test?

francoism90 commented 5 years ago

It would be great if a job (e.g. Spatie\MediaLibrary\Jobs\PerformConversions) could be controlled by the user's queue driver - adding Horizon features to set job rate limiting, max. simultaneously procs for jobs, etc. Or is this already possible? :)

I'll try to reproduce this and submit a PR.

francoism90 commented 5 years ago

@brendt I think I have solved the throttle issue:

<?php

namespace App\Support\MediaLibrary\Jobs;

use Illuminate\Bus\Queueable;
use Spatie\MediaLibrary\Models\Media;
use Illuminate\Queue\SerializesModels;
use Illuminate\Queue\InteractsWithQueue;
use Spatie\MediaLibrary\FileManipulator;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Support\Facades\Redis;
use Spatie\MediaLibrary\Conversion\ConversionCollection;

class PerformConversions implements ShouldQueue
{
    use InteractsWithQueue, SerializesModels, Queueable;

    /** @var \Spatie\MediaLibrary\Conversion\ConversionCollection */
    protected $conversions;

    /** @var \Spatie\MediaLibrary\Models\Media */
    protected $media;

    /** @var bool */
    protected $onlyMissing;

    public function __construct(ConversionCollection $conversions, Media $media, $onlyMissing = false)
    {
        $this->conversions = $conversions;

        $this->media = $media;

        $this->onlyMissing = $onlyMissing;
    }

    /**
     * Determine the time at which the job should timeout.
     *
     * @return \DateTime
     */
    public function retryUntil()
    {
        return now()->addHour(1);
    }

    /**
     * @return mixed
     */
    public function handle()
    {
        Redis::funnel('medialibrary:conversions')->limit(1)->then(function () {
            app(FileManipulator::class)->performConversions($this->conversions, $this->media, $this->onlyMissing);

            return true;
        }, function () {
            return $this->release(10);
        });
    }
}
/*
     * Here you can override the class names of the jobs used by this package. Make sure
     * your custom jobs extend the ones provided by the package.
     */
    'jobs' => [
        'perform_conversions' => App\Support\MediaLibrary\Jobs\PerformConversions::class,
        'generate_responsive_images' => Spatie\MediaLibrary\Jobs\GenerateResponsiveImages::class,
    ],
spatie-bot commented 4 years ago

Dear contributor,

because this issue seems to be inactive for quite some time now, I've automatically closed it. If you feel this issue deserves some attention from my human colleagues feel free to reopen it.

martindrapeau commented 2 years ago

@francoism90 in which file can I override the perform_conversions class? I want to specify a number of tries for this job. Maybe there is an easier way to do that than overriding the class?

/*
     * Here you can override the class names of the jobs used by this package. Make sure
     * your custom jobs extend the ones provided by the package.
     */
    'jobs' => [
        'perform_conversions' => App\Support\MediaLibrary\Jobs\PerformConversions::class,
        'generate_responsive_images' => Spatie\MediaLibrary\Jobs\GenerateResponsiveImages::class,
    ],
francoism90 commented 2 years ago

@martindrapeau I'm doing something different nowadays. I simple listen for MediaHasBeenAdded/MediaHasBeenUpdated and do my own logic (conversions, jobs, services).

martindrapeau commented 2 years ago

Ok. I ended up setting a number of tries in the Horizon supervisor. Merci François. --Martin

On Tue, Mar 22, 2022 at 3:24 AM François M. @.***> wrote:

@martindrapeau https://github.com/martindrapeau I'm doing something different nowadays. I simple listen for MediaHasBeenAdded/ MediaHasBeenUpdated and do my own logic (conversions, jobs, services).

— Reply to this email directly, view it on GitHub https://github.com/spatie/laravel-medialibrary/issues/1538#issuecomment-1074822595, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZNXEBFXD543UUBKNUD5LVBFYRFANCNFSM4IPGS3DA . You are receiving this because you were mentioned.Message ID: @.***>