Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Job ID #896

Closed
endymonium opened this issue May 3, 2013 · 9 comments
Closed

Custom Job ID #896

endymonium opened this issue May 3, 2013 · 9 comments

Comments

@endymonium
Copy link

Hi,

I needed to set the job id manually to able to bind a worker uniquely to a models instance.

The change I did (see https://github.com/endymonium/sidekiq/commit/5feb15a097fae2f3afec4db2ba54c70bc3f3e9c8) is not 100% clean, but perhaps something similar could be implemented? Or is there a better way to archive this?

Example

class TestWorker
  include Sidekiq::Worker

  def perform(args)
    (0..5).each do |i|
      puts 'Run ' + i.to_s
      sleep 10
    end
  end
end

then

TestWorker.perform_async(jid: 'some_id')
@mperham
Copy link
Collaborator

mperham commented May 3, 2013

I don't follow your example. How is the jid relevant to perform?

@endymonium
Copy link
Author

The jid: 'some_id' is passed as an argument to perform and you'll get an wrong number of arguments error if you don't do that ... didn't dig so deep into sidekiq to find out exactly why :)

@mperham
Copy link
Collaborator

mperham commented May 3, 2013

Your model instance's ID has nothing to do with Sidekiq's job ID. You just want to do:

TestWorker.perform_async(instance.id)
def perform(id)
  instance = Model.find(id)
  instance.do_something
end

Right or am I still confused?

@endymonium
Copy link
Author

No, I mean something else. Sorry for being so confusing :) Here's my use case:

  1. Start worker with a job id which corresponds to the instance.id
  2. Check regular on that worker using the instance.id, e.g.
class SomeModel < ActiveRecord::Base
  attr_accessor :worker_working
  attr_accessible :id

  # Returns true of a sidekiq worker is currently active for this id
  def worker_working?
    working = false
    workers = Sidekiq::Workers.new
    workers.each do |name, work|
      if work['payload']['jid'] == self.id
        working = true
      end
    end
    working
  end

end

In my ruby code I can then easily control the flow with if SomeModel.worker_working? do ... or prevent the start of a new worker (this is similar to #4) ... I hope I could make myself understandable now?

@mperham
Copy link
Collaborator

mperham commented May 3, 2013

It looks like you're trying to determine when a job is done? There are two ways to do this:

  1. Various job status plugins, linked in the Related Projects wiki page.
  2. Sidekiq Pro's batch feature can track one or more jobs and inform you when they are complete.

@mperham mperham closed this as completed May 3, 2013
@endymonium
Copy link
Author

I checked the status plugins, the issue is the same: if I didn't miss it, you cannot specify the job id manually, it is generated while calling perform_async.

Let's assume the following work flow:

  1. The user calls a web page which starts a worker which needs 1 hour to finish (extreme example)
  2. The webrequest is finished, the user goes somewhere else or closes the browser all together
  3. Half an hour later, the user wants to check on the status of that specific worker. For that we need it's job id.

Currently the job id is a return value from perform_async. Therefore to be able to search for that worker half an hour later, we would have needed to persist the id somewhere in the database at step 1.

I don't want to do that, I want to deduct the job id dynamically (in my case from the instance id). And for that I need to be able to control the name if the jid.

I think that is a valid use case, because the convention over configuration approach helps you to keep you code tidy.

Or am I missing something here completely?

@mperham
Copy link
Collaborator

mperham commented May 3, 2013

The Job ID is not meant to be user-configurable; you are supposed to persist the returned jid if you want to poll or check for status later.

The edge case you are missing is if there's a problem pushing the job to Redis. Right now this returns the jid or nil. In your case, you could poll for it in (3), not realizing it failed to push in the first place.

@endymonium
Copy link
Author

Ah ok, now I understand the reasoning behind this.

But imho a custom job id and the error handling is not 100% connected with each other. Meaning: if I specify a custom id, the perform_async call can stil return nil in case of a Redis push error. And as I'm still in a synchronous flow at that point of time, I can directly notify the user that an error has occurred/ change the flow accordingly.

But I agree this is a very specific feature request ... I'll be quite now and just use my fork :)

@omarcruzpantoja
Copy link

omarcruzpantoja commented Jul 11, 2022

@endymonium I think i follow what you’re looking to do. Basically just adding some sort either

  1. Prefix parameter
  2. Specific id.
    Probably passing specific id might not be “great” idea considering there could be race conditions on the table id (on user creation for example)

The issue that were looking to “fix” is basically being able to find jobs easily, without having to store the random hex in the database.

If i start a job, and I want to know its status, I must somehow keep that randomhex somewhere, within the scope of the process.
With a customized jid, it’ll be easier to know how to search for the status for other processes outside of the original scope.

im assuming your fork changes the jid generation in https://github.com/mperham/sidekiq/blob/67daa7a408b214d593100f782271ed108686c147/lib/sidekiq/job_util.rb#L49

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants