From df5a911168f2c16366a0d0cc69d8cbb9a1b3e1ff Mon Sep 17 00:00:00 2001 From: Peter Deltchev Date: Mon, 4 Jan 2016 08:42:30 -0800 Subject: [PATCH] Fixed an explosion of queued encoding jobs. --- app/Commands/EditAlbumCommand.php | 12 +++++----- app/Jobs/EncodeTrackFile.php | 37 ++++++++++++++++++++++++------- app/Models/Album.php | 15 +++++++++++++ app/Models/Track.php | 10 ++++----- app/Models/TrackFile.php | 27 +++++++++++++++++----- app/Traits/TrackCollection.php | 1 - 6 files changed, 76 insertions(+), 26 deletions(-) diff --git a/app/Commands/EditAlbumCommand.php b/app/Commands/EditAlbumCommand.php index fd8a68cf..63cc6267 100644 --- a/app/Commands/EditAlbumCommand.php +++ b/app/Commands/EditAlbumCommand.php @@ -22,14 +22,16 @@ namespace Poniverse\Ponyfm\Commands; use Poniverse\Ponyfm\Models\Album; use Poniverse\Ponyfm\Models\Image; -use Illuminate\Support\Facades\Auth; -use Illuminate\Support\Facades\DB; -use Illuminate\Support\Facades\Validator; +use Auth; +use DB; +use Validator; class EditAlbumCommand extends CommandBase { private $_input; + /** @var int */ private $_albumId; + /** @var Album */ private $_album; function __construct($trackId, $input) @@ -88,10 +90,6 @@ class EditAlbumCommand extends CommandBase $this->_album->syncTrackIds($trackIds); $this->_album->save(); - Album::where('id', $this->_album->id)->update([ - 'track_count' => DB::raw('(SELECT COUNT(id) FROM tracks WHERE album_id = ' . $this->_album->id . ')') - ]); - return CommandResponse::succeed(['real_cover_url' => $this->_album->getCoverUrl(Image::NORMAL)]); } } diff --git a/app/Jobs/EncodeTrackFile.php b/app/Jobs/EncodeTrackFile.php index 276802e8..24755e86 100644 --- a/app/Jobs/EncodeTrackFile.php +++ b/app/Jobs/EncodeTrackFile.php @@ -24,11 +24,9 @@ namespace Poniverse\Ponyfm\Jobs; use Carbon\Carbon; use DB; use File; -use Illuminate\Support\Facades\Config; -use Illuminate\Support\Facades\Log; -use OAuth2\Exception; +use Config; +use Log; use Poniverse\Ponyfm\Exceptions\InvalidEncodeOptionsException; -use Poniverse\Ponyfm\Jobs\Job; use Illuminate\Queue\SerializesModels; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Contracts\Bus\SelfHandling; @@ -74,10 +72,23 @@ class EncodeTrackFile extends Job implements SelfHandling, ShouldQueue throw new InvalidEncodeOptionsException("Master files cannot be encoded unless we're generating a lossless master file during the upload process."); } + // don't start this job if the file is already being processed or if it's still valid + if ( + in_array($trackFile->status, [TrackFile::STATUS_PROCESSING_PENDING, TrackFile::STATUS_PROCESSING]) || + !$trackFile->is_expired + ) { + $this->delete(); + return; + } + $this->trackFile = $trackFile; $this->isExpirable = $isExpirable; $this->isForUpload = $isForUpload; $this->autoPublishWhenComplete = $autoPublish; + + // "lock" this file for processing + $this->trackFile->status = TrackFile::STATUS_PROCESSING_PENDING; + $this->trackFile->save(); } /** @@ -87,9 +98,19 @@ class EncodeTrackFile extends Job implements SelfHandling, ShouldQueue */ public function handle() { + // Sanity-check: was this file just generated, or is it already being processed? + if ($this->trackFile->status === TrackFile::STATUS_PROCESSING) { + Log::warning('Track file #'.$this->trackFile->id.' (track #'.$this->trackFile->track_id.') is already being processed!'); + return; + + } elseif (!$this->trackFile->is_expired) { + Log::warning('Track file #'.$this->trackFile->id.' (track #'.$this->trackFile->track_id.') is still valid! No need to re-encode it.'); + return; + } + // Start the job $this->trackFile->status = TrackFile::STATUS_PROCESSING; - $this->trackFile->update(); + $this->trackFile->save(); // Use the track's master file as the source if ($this->isForUpload) { @@ -132,7 +153,7 @@ class EncodeTrackFile extends Job implements SelfHandling, ShouldQueue // Insert the expiration time for cached tracks if ($this->isExpirable) { $this->trackFile->expires_at = Carbon::now()->addMinutes(Config::get('ponyfm.track_file_cache_duration')); - $this->trackFile->update(); + $this->trackFile->save(); } // Update file size @@ -140,7 +161,7 @@ class EncodeTrackFile extends Job implements SelfHandling, ShouldQueue // Complete the job $this->trackFile->status = TrackFile::STATUS_NOT_BEING_PROCESSED; - $this->trackFile->update(); + $this->trackFile->save(); if ($this->isForUpload) { if (!$this->trackFile->is_master && $this->trackFile->is_cacheable) { @@ -171,6 +192,6 @@ class EncodeTrackFile extends Job implements SelfHandling, ShouldQueue { $this->trackFile->status = TrackFile::STATUS_PROCESSING_ERROR; $this->trackFile->expires_at = null; - $this->trackFile->update(); + $this->trackFile->save(); } } diff --git a/app/Models/Album.php b/app/Models/Album.php index a53356b7..6666a4eb 100644 --- a/app/Models/Album.php +++ b/app/Models/Album.php @@ -388,4 +388,19 @@ class Album extends Model { return 'album-' . $this->id . '-' . $key; } + + /** + * The number of tracks in an album will always be in sync. + * + * @param array $options + * @return bool + */ + public function save(array $options = []) { + $this->recountTracks(); + return parent::save($options); + } + + protected function recountTracks() { + $this->track_count = $this->tracks->count(); + } } diff --git a/app/Models/Track.php b/app/Models/Track.php index 05dbe981..f45172a6 100644 --- a/app/Models/Track.php +++ b/app/Models/Track.php @@ -389,11 +389,11 @@ class Track extends Model ], 'url' => $track->url, 'slug' => $track->slug, - 'is_vocal' => (bool)$track->is_vocal, - 'is_explicit' => (bool)$track->is_explicit, - 'is_downloadable' => (bool)$track->is_downloadable, - 'is_published' => (bool)$track->isPublished(), - 'published_at' => $track->published_at->format('c'), + 'is_vocal' => $track->is_vocal, + 'is_explicit' => $track->is_explicit, + 'is_downloadable' => $track->is_downloadable, + 'is_published' => $track->isPublished(), + 'published_at' => $track->isPublished() ? $track->published_at->format('c') : null, 'duration' => $track->duration, 'genre' => $track->genre != null ? diff --git a/app/Models/TrackFile.php b/app/Models/TrackFile.php index ef263ee6..265cdc5c 100644 --- a/app/Models/TrackFile.php +++ b/app/Models/TrackFile.php @@ -37,12 +37,13 @@ use File; * @property \Carbon\Carbon $updated_at * @property boolean $is_cacheable * @property boolean $status - * @property string $expires_at + * @property \Carbon\Carbon $expires_at * @property integer $filesize * @property-read \Poniverse\Ponyfm\Models\Track $track * @property-read mixed $extension * @property-read mixed $url * @property-read mixed $size + * @property-read mixed $is_expired */ class TrackFile extends Model { @@ -50,11 +51,22 @@ class TrackFile extends Model const STATUS_NOT_BEING_PROCESSED = 0; const STATUS_PROCESSING = 1; const STATUS_PROCESSING_ERROR = 2; + const STATUS_PROCESSING_PENDING = 3; - - public function track() - { - return $this->belongsTo('Poniverse\Ponyfm\Models\Track')->withTrashed(); + protected $appends = ['is_expired']; + protected $dates = ['expires_at']; + protected $casts = [ + 'id' => 'integer', + 'track_id' => 'integer', + 'is_master' => 'boolean', + 'format' => 'string', + 'is_cacheable' => 'boolean', + 'status' => 'integer', + 'filesize' => 'integer', + ]; + + public function track() { + return $this->belongsTo(Track::class)->withTrashed(); } /** @@ -93,6 +105,11 @@ class TrackFile extends Model } } + public function getIsExpiredAttribute() { + return $this->attributes['expires_at'] === null || + $this->attributes['expires_at']->isPast(); + } + public function getFormatAttribute($value) { return $value; diff --git a/app/Traits/TrackCollection.php b/app/Traits/TrackCollection.php index 442649bf..9b4bd938 100644 --- a/app/Traits/TrackCollection.php +++ b/app/Traits/TrackCollection.php @@ -102,7 +102,6 @@ trait TrackCollection foreach ($trackFiles as $trackFile) { /** @var TrackFile $trackFile */ - if (!File::exists($trackFile->getFile()) && $trackFile->status == TrackFile::STATUS_NOT_BEING_PROCESSED) { $this->dispatch(new EncodeTrackFile($trackFile, true)); }