Queue only runs 0 or 1 task per refresh interval per worker

[danpalmer]

Describe the bug

The queues package is only able to run up to 1 task per refresh interval, per worker. For example, a system configured with 1 worker and a 10 second refresh interval, running tasks that take 1 second, cannot be utilised beyond 10%, and cannot run more than 1 task every 10 seconds.

To Reproduce

In a project with queues and a task configured...

1. Set the number of workers to 1
2. Set the refresh interval to 10 seconds
3. Enqueue 10 tasks.

Expected behaviour
All 10 tasks complete one after the other with effectively zero gap between them.

Actual behaviour
The first task completes, and the queue waits until 10 seconds have elapsed before running the next.

Environment

queues 1.12.1
vapor-queues-fluent-driver 3.0.0-beta1

Additional context

I'm not sure whether this is worth filing on the fluent driver repo as well. I think there are two possible scenarios here:

1. queues intends that the pop method in the driver API blocks until a job is available, likely intended for use with BRPOPLPUSH/etc on Redis. Therefore making this a "bug" in vapor-queues-fluent-driver or the queues documentation.
2. (and/or) This is unintentional behaviour in queues, and it should instead re-pop if any job is run until there is nothing.

While the intention of refreshInterval isn't documented (that I can tell), I think it's most reasonable and would be most expected by those who have used other queueing systems, that it be a period to wait when there is no available work so the tuning of this value only impacts the polling of storage and does not impact the utilisation of queue workers.

[danpalmer]

@0xTim hey, what do you think of this? Is this a bug in the queues package, or a bug in the fluent driver due to a mismatch in expectations. Or, am I just using it wrong!

[danpalmer]

@0xTim @gwynne hey just a quick ping as it has been a while. I've looked at this in more detail and I'm fairly sure that this is a major performance issue with this package, at least for the Fluent driver, and fixing it looks like it might require significant work in the main Queues package.

I could be wrong, and would love to be corrected here as it would save a lot of work!

[danpalmer]

First pass fix in #134.

[0xTim]

@danpalmer sorry for the delay in this! Got buried in GH notifications. I've had a quick look at the PR and it makes sense, but well need to update it to avoid breaking the API