Count in QC::Queue returns scheduled jobs too

[jipiboily]

This is a breaking change from before, at least if you were using queue_classic-later, for other people, it's just a new thing.

We should have a count per queue with scheduled jobs, but also one for the jobs that are ready to be processed. As we already pushed out a RC, it's too late to change the behavior of this one IMHO but we could add two news ones for the next RC maybe?

My use case is for Hirefire: https://github.com/meskyanichi/hirefire-resource/blob/bc0b5139602a7142547d6768a4ad3f4ef7622d3f/lib/hirefire/macro/qc.rb

Thoughts @senny?

[jipiboily]

Related to the hirefire-resource issue hirefire/hirefire-resource-ruby#24.

[mrrooijen]

Hi JP,

Could you further elaborate on why this would be an issue? From what I understand you're adding support for:

• Scheduling jobs to run in the future
• Multiple Queues

The count method currently includes all jobs (which is fine), but in the future will also include jobs that are scheduled to run in the future, which isn't the desired behavior.

Can't we solve it in a similar way to how we solved the DJ macro which also supports scheduled jobs and multiple queues? For example, see this simplified DJ macro:

module HireFire::Macro::Delayed::Job
  extend self

  def queue(*queues)
    # Queues to include in the count operation. Empty array means all queues.
    queues.flatten! 

    # Start criteria chain.
    c = ::Delayed::Job.where(failed_at: nil)

    # Ensure no scheduled jobs are included.
    c = c.where("run_at <= ?", Time.now.utc)

    # Don't scope to certain queues unless explicitly specified.
    c = c.where(queue: queues) unless queues.empty? 

    # Return the number of jobs based on the above criteria.
    c.count
  end
end

# Count all queues, minus jobs that are scheduled to run in the future.
HireFire::Macro::Delayed::Job.queue 

# Only counts jobs in the "default" queue, minus jobs that are scheduled to run in the future.
HireFire::Macro::Delayed::Job.queue("default")

# Only counts jobs in the "default" and "mailer" queues, minus jobs that are scheduled to run in the future.
HireFire::Macro::Delayed::Job.queue("default", "mailer")

# etc.

Wouldn't something like this work with QC?

[jipiboily]

Sure, that would work! I opened this issue mostly for other people that would face that. I will look at a proper fix and possibly send a PR at some point too.

[mrrooijen]

Cool. Let me if you need any help.

[senny]

The count method currently includes all jobs (which is fine), but in the future will also include jobs that are scheduled to run in the future, which isn't the desired behavior.

As I see it, the current count method has a very vague meaning. There is no rDoc comment explaining what count you'll get back. With the addition of scheduled jobs we need to better define it's meaning. With only the name count it's behavior could both mean "1.) All the jobs in this queue" or "2.) All jobs in this queue ready to be processed".

We should have an API for both 1.) and 2.). Just from the name count I'd jump to the conclusion that it's counting all jobs including scheduled ones but I don't have a strong opinion on that one. Depends on what names we come up with.

[mrrooijen]

@senny sorry, let me rephrase that: "which isn't the desired behavior for HireFire.".

We should have an API for both 1.) and 2.). Just from the name count I'd jump to the conclusion that it's counting all jobs including scheduled ones but I don't have a strong opinion on that one. Depends on what names we come up with.

I agree that count should return the job count for all jobs, including jobs that are scheduled to run at a later date/time. I've never worked with QC myself so I don't know much about the library.

For it to work with HF there will need to be a way to limit the count scope to:

• Specific Queues (optionally)
• Jobs that are scheduled to run right now (i.e. run_at <= Time.now)

With HF you actually never want to include jobs that aren't scheduled to run yet, so you end up with only two scenarios:

• Count jobs across all queues that are scheduled to run now.
• Count jobs for one or more queues that are scheduled to run now.

See code example in my first comment.

[senny]

@meskyanichi 👍

[ukd1]

Closing as #255