Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Don't allow a job to be executed again immediately after execution #23

Merged
merged 1 commit into from Sep 5, 2012

Conversation

Projects
None yet
2 participants
Contributor

mateusdelbianco commented Sep 3, 2012

No description provided.

@jayniz jayniz commented on an outdated diff Sep 5, 2012

lib/resque-loner/helpers.rb
@@ -21,7 +21,11 @@ def self.mark_loner_as_queued(queue, item)
def self.mark_loner_as_unqueued(queue, job)
item = job.is_a?(Resque::Job) ? job.payload : job
return unless item_is_a_unique_job?(item)
- redis.del(unique_job_queue_key(queue, item))
+ unless (ttl=persist_ttl(item)) == 0
@jayniz

jayniz Sep 5, 2012

Contributor

Hmm, you interpret the ttl as a "block" window after the execution of a job? I.e. with a ttl of 10, no same job can be enqueued until 10 seconds after the previous job was finished? I think that the meaning of ttl was that a job should only be marked as "enqueued already" for a specific time. I.e. mark the job as not queued after 10 seconds, regardless of wether or not it was run already.

Could you make sure we don't change the way resque loner works here, please? :-)

Thanks for sharing your work!

Contributor

mateusdelbianco commented Sep 5, 2012

I was looking for a plugin to do exactly that, a "lock after execution". And I didn't mean to change the way resque-loner works. That's why I created another variable to hold the "persist_ttl" value, making the original loner_ttl working as it should.

Maybe we could rename the "persist_ttl" instance variable to better express what it does? Something like loner_lock_ttl, loner_lock_after_execution_ttl...

Contributor

jayniz commented Sep 5, 2012

Ah, okay, I'm a bit busy and perhaps rushed over your pull request too quickly :-) Apologies. I like the loner_lock_after_execution_ttl or loner_lock_after_execution_period!

Contributor

mateusdelbianco commented Sep 5, 2012

Using loner_lock_after_execution_period, thanks @kunalmodi for the initial persist code.

@jayniz jayniz added a commit that referenced this pull request Sep 5, 2012

@jayniz jayniz Merge pull request #23 from mateusdelbianco/persist-loner-status
Don't allow a job to be executed again immediately after execution
6ffc295

@jayniz jayniz merged commit 6ffc295 into resque:master Sep 5, 2012

Contributor

jayniz commented Sep 5, 2012

Thanks @mateusdelbianco! Merged. If I don't push a new resque-loner release in the next week. If not, then I forgot about it and should be blamed for all the bad things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment