search

jessetane / queue

Asynchronous function queue with adjustable concurrency
MIT License
764 stars 66 forks source link

Discussion: job-based timeouts, support for dropping jobs, and max-queue length? #59

Closed joelgriffith closed 5 years ago

joelgriffith commented 5 years ago

Some thoughts here as I've used this module for quite a bit, and noticed a few things:

  1. It'd be nice if we could specify/override timeout's on a per-job basis. This obviously would change the simple API, but is nearly impossible to implement in user-land. The only way I think of is adding a property to the job of something like timeout, and queue can either use that or default to the timeout specified during initialization.

  2. In my use case, incoming http requests are using this module as a way to establish a concurrency and queue those over. Every now and then a request that is dropped still has an item in the queue, and needs to be dropped since the underlying request failed. I've got this monkey-patched in my project, but it'd be more elegant if queue supported something like queue.remove(job) or it implemented queue.filter, which is what I've done.

  3. It'd be nice to establish a threshold on how many jobs can be queued. This might be tough to add as an API since there's a few ways jobs find themselves into the queue, but having a ceiling on "pending" operations would be useful. Again, something that I've implemented in my project, but would be one last thing to maintain.

Thanks!

jessetane commented 5 years ago

Cool, thanks for sharing. Some quick takes below:

  1. The idea of a per-job timeout property seems interesting, happy to look at a patch
  2. To remove jobs, I typically use splice, but other existing array methods like filter could be interesting too if they don't bloat things too much?
  3. I guess for this I would say it's so easy to do in userland (if (q.length < 5) q.push(job)) it seems unnecessary to add here, but happy to look at patches
joelgriffith commented 5 years ago

Thanks for the feedback on this. I’m going to close this issue now that #1 is merged, and I agree with your points on #2 and #3. I did attempt a filter patch, but recalled that the filter method doesn’t mutate the array it operates on, so I think splice is the best mechanism going forward.

Thanks again!

alolis commented 2 years ago

@jessetane, @joelgriffith can you please share your approaches regarding splice and how you actually find the index of the item you are interested in removing?

EDIT: May I assume with indexOf ?