carrierwave_backgrounder
carrierwave_backgrounder copied to clipboard
When destroy a model, same queue is repeatedly processed
When I destroyed a model, I encountered the problem that same sidekiq queue was repeatedly processed in a very short time, and finally sidekiq.log
file became very huge size.
I thought this was because enqueue_#{column}_background_job
was kicked by after_commit
, enqueued my resize job, my resize job always failed and called when_not_ready
because record not found, and retried the same job repeatedly.
My resize job is like this (same as README.md
)
class MyResizeJobWorker < ActiveJob::Base
include ::CarrierWave::Workers::ProcessAssetMixin
def when_not_ready
retry_job
end
end
I know I can avoid this problem by deleting retry_job
method from my resize job class.
But I think enqueing callback should be kicked only when record was saved or updated, not destroyed.
What do you think about it?
I'm also experiencing this. After looking at the code it's basically just swallowing the exceptions that ActiveRecord / Mongoid are throwing
https://github.com/lardawge/carrierwave_backgrounder/blob/master/lib/backgrounder/workers/base.rb#L16
After looking at the code it's basically just swallowing the exceptions that ActiveRecord / Mongoid are throwing
@kyledecot Isn't that the correct behaviour? If versions start processing, and the record gets deleted in the meanwhile, then the background job shouldn't raise an error, right? I mean, there is nothing that the background job can do at that point (except maybe delete the processed files, since they won't be used anymore).
@janko-m perhaps but then how would one handle the case this issue describes wherein the record is destroyed before the background job is processed (resulting in an infinite loop of retries when using the when_not_ready
hook). This issue in particular caused an enormous spike in redis keys being set which is the only way that I noticed the behavior in the first place as the exceptions were being swallowed.
@kyledecot Oh, I understand what you mean now. It's surprising that carrierwave_backgrounder has this behaviour, since the background job is spawned in an after_commit
hook, which means that the record will definitely be there in the background job. And if it's not there, then it must mean that it was deleted, like it is in your case, and then it will never be present again.
From time to time I keep checking this repo to see if there is something that I might have missed in Shrine, but it's strange to see carrierwave_backgrounder (and delayed_paperclip) still struggling with things that I already solved in Shrine in the early days. Not saying that it's easy, it was definitely very hard and took a lot of time, but I knew I had to solve these problems if I wanted backgrounding to be reliable. Just putting it out there in case it pokes somebody's interest.
@kyledecot How did you stop it? I have a process running shit loads of jobs from a deleted resource...
@davidwessman I basically just re-raised the exception:
class AvatarJob < ActiveJob::Base
include ::CarrierWave::Workers::ProcessAssetMixin
queue_as :carrierwave
def when_not_found
raise ActiveRecord::RecordNotFound
end
def when_not_ready
retry_job
end
end
I see that you've submitted a pull request to set the default job worker to retry_job
on when_not_ready
. This however was the source of my problem as a deleted avatar in my case would never be ready and it resulted in an infinite queueing of jobs.
👍 @kyledecot
Thank you for telling me!
Where did you find the when_not_found
method?
@davidwessman I forked the project and implemented it myself (#248) . So far it's been working well enough but it definitely doesn't seem like the most elegant solution.
Oh! @kyledecot Would be good as a feature...
The issue still persists. Right now i have an issue when deleting images that are still processed and I find that the only way to prevent this is to query the sidekiq queue, find the job by class and args and delete. Really slow and not recommended probably. Has anyone found a way around this?
I know this is old but want to update in case this comes up via a google search.
I am preparing to release a 1.0 version of this in the coming week. In doing so, I am attempting to add support for ActiveJob natively. This has led me down the rabbit hole of trying to understand what the idea behind the when_not_ready
hook was used for.
Here's the deal, there is no use case for it. We are using after_commit
. This means that by the time the job runs, there should be no issue with running it. Please chime in if I am missing a use case.
In the case where the record does not exist, it should fail silently and not retry, which it currently does.
There is a 1.0-beta branch for the curious. As always, I advise you to read through the docs first. There have been a few things removed that are just not useful at the moment. The idea here is to make it dead simple to process and store files in the background.
Also, it should be much easier to test new features as I have added a rails app in the spec directory to test against.