asynq: [QUESTION] i think set stop in run function is not a good idea

Closed via #135

@zsiec Awesome to hear your feedback! Thank you.

I’m currently drafting a PR for this change, which includes a few breaking changes (I’ll make sure to document the upgrade paths in CHANGELOG for those who are on the current version).

+1 to exporting these. Some weeks ago I forked and exposed start/stop myself but have now stumbled on this thread and would be happy to remove my fork and use this once it’s ready!

My current implementation (importing my fork) handles the syscall termination signals for the workers as I was already handling them for other things before using asynq and it’s helpful to me to have my graceful shutdown logic collocated. It was also trivial to implement a shutdown timeout external to this library.

Thanks for your work on this very usable project, looking forward to seeing these updates!

It’s fine. breaking change needs to thinking more and more.

Looking forward to your final design.

Yep, just exporting just Start and Stop makes sense to me (chose Stop because of the symmetry with Start).

So effectively, if user uses Start and Stop instead of Run they don’t get some nice signal handling but I guess that’s okay. If we get feature request to export Quiet(i.e. stop processing new tasks), maybe we’ll add that later.

I’ll get started on making these changes and ping you in the PR 👍

I see. I can see why this could be confusing. Let me try to explain.

First Question

since we have Stop method, what does Shutdown do ? this method make me think the process will exist.

Asynq took a lot of ideas from Sidekiq and they use OS signals to handle graceful shutdown (video).

Currently, when you start background workers with bg.Run it waits for signals. Asynq handles TERM, INT, and TSTP signals. If you send TSTP signal to the process, asynq will stop processing new tasks but doesn’t exit. This is documented in Sidekiq here.

If you send TERM or INT signal to the process, asynq will wait for few seconds to let active workers finish its tasks but if they don’t finish within that timeout, asynq will push the tasks back to Redis and exit the process. This is documented in Sidekiq here.

My intention was to name the first operation Stop and the second one Shutdown. But I can see why Stop is a bit confusing. Looking at sidekiq doc, maybe we can name it bg.Quiet()?

Does that makes sense?

Second Question

bg.Handler can receive options to custom each task handler, i think its important. i can’t find this feature in current design, the option like Queues and Concurrency seems should be task level instead of global level

I agree with the Concurrency per Handler (i.e. Concurrency per task type). For example, if you have a task to use third-party API and if they have a limit on how many concurrent API calls they can handle, this is definitely useful. Am I understanding you correctly here?

I’m not sure if we can add an option to specify Queues per Handler though. gocraft/work has a different design in that they have one-to-one mapping between job-type and queue. Asynq’s design is similar to sidekiq. It allows for multipe queues with different priority but any task can be put on any queue. So I’m not sure if we can create Queue option for a Handler. (please let me know if I’m not understanding your point correctly).

1. the idea from net/http seems good, but the functions name NewServer is confused.If i did not read the doc, i would think it’s a server to receive request and produce a job. other names like Serve, NewServeMux also confused

2. Handler is part of Config: i prefer the api like gin

bg := asynq.NewBackgroud(...)
bg.Use(middleware)
bg.Register(handler1)
bg.Register(handler2)