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

New library SAQ #283

Closed
tobymao opened this issue Jan 6, 2022 · 23 comments
Closed

New library SAQ #283

tobymao opened this issue Jan 6, 2022 · 23 comments

Comments

@tobymao
Copy link
Contributor

tobymao commented Jan 6, 2022

I made a couple of PR's that have been unaddressed, so I decided to create my own async queue that I feel improves upon ARQ called SAQ.

Perhaps these features could be incorporated into ARQ.

  1. Leveraging BLMOVE / RPOPLPUSH / NOTIFY. SAQ has latencies of a couple milliseconds when starting a job whereas ARQ uses polling so is much slower.

  2. A simple web interface

  3. Heartbeat monitor for stuck jobs

  4. More robust error handling like stack traces, sweeping stuck jobs, and not incrementing of retries on cancel.

  5. before and after job hooks

Some simple benchmarks comparing SAQ and ARQ.

@samuelcolvin
Copy link
Member

samuelcolvin commented Jan 6, 2022

This library is not unmaintained. I use it all the time and really want to devote some time to it. It's just that I have an all consuming job, a baby and other open source projects to maintain. But I'll try to put some time aside to go through PRs soon.

SAQ looks interesting, I'll have a look at it soon.


More generally, with issues like this. I understand you want to promote your library, but issues worded like this (particularly the title) are a little anti-social IMHO.

Imagine SAQ get's a bit popular, you maintain and update it for 6 years, but then you're busy and you forget to reply to some pull requests for a few months. You might well feel a little frustrated if someone came along and created an issue "Unmaintained? New library XAQ"?

@JonasKs
Copy link
Collaborator

JonasKs commented Jan 6, 2022

I understand both of you in this situation, but I truly believe tobymao did this with the best intention - attempting to contribute here before he created a new library, and he definetly give arq a lot of credit.

And there is this important quote:

Perhaps these features could be incorporated into ARQ.

@samuelcolvin
Copy link
Member

Ye I see that.

And yes, when I get time to put some TLC into arq some or all of those features would be good.

@sondrelg
Copy link
Contributor

sondrelg commented Jan 6, 2022

Not trying to antagonize you in any way here @samuelcolvin, and have completely understand you're stretched for time - but would it be possible to give a (non-committing) estimate of when you think you'll have time to address some of the issues here?

@samuelcolvin
Copy link
Member

I'll try to have a look this weekend unless pydantic's latest release needs a patch, in which case I might need to do that instead.

@tobymao
Copy link
Contributor Author

tobymao commented Jan 6, 2022

@samuelcolvin , first and foremost, I’d like to apologize to you. I didn’t mean to upset you. Also congratulations on your new child. I know how tough that can be.

I appreciate the work you’ve done on your library. However, many people depend on your library, like myself and when there’s no response for months, people look for other options. If you’re constrained on time, could you perhaps promote other contributors so that improvements can continue in your absence?

Again, I appreciate all you’ve done and made this issue because I felt like I had no choice, and I wanted to give others who depend on this library another option.

@ovresko
Copy link

ovresko commented Jan 23, 2022

Arq could be big, if only some design decisions could be reconsidered while it is small project still.
I think the big drawback is polling the fact that each job take at least 1+n redis request to return results, we have a system avreging 1000 job in any moment, imaging 1000 redis.get request each 500ms where half of them run 2,3 or more get request to find a result (poll_delay is not enough for dynamic job loads) maybe a batch read is more effecient where one redis.scan is used to get all finished jobs (and attach each result by id)

@samuelcolvin
Copy link
Member

@tobymao, thanks for your kind words. I'd be happy to add you as a contributors if you'd like? Obviously we'd need to discuss changes before merging anything non-trivial.

On the general point of having time to improve arq (and other libraries), I'm hoping to make some big changes to my situation this year that allow (even require) me to spend much more time on open source. Until then, I can't do much more than fight the fires.

@ovresko I partially agree with you - arq's performance could use some work, but actually polling isn't the big problem (unless you're using job.result() masses.

There are two things (I think) that harm arq's performance, one easy to fix, one hard, one (I think) impossible:

easy

when I built this version of arq I didn't know about setnx (not sure if it's been added since I first learnt about redis, or if I just missed it), so there's a lot of watch-do-catch-watcherror than could be replaced by a single setnx

hard

Polling for results could be harmful if you do a lot of it, we could replace this with pub/sub. It's a fair bit of work, but not impossible.

unfixable

Most queuing libraries based on redis use some variant of BLPOP, arq used blpop before v.16. The problem with that approach is that (like the name says) it removes the job description from redis at the beginning of the job. If the job fails unexpectedly, or the worker is shut down, the job is gone.

I made the very intentional decision not to remove the job from the queue until it's done. That way, if the job fails or is cancelled unexpected the job is still in the queue, when the "this job is in progress" key expires the job will be run again.


Overall:

  1. Think arq's performance can be improved drastically with relatively few breaking changes
  2. There will have to be breaking changes to support aioredis v2, see support aioredis v2 #259 (comment)
  3. I have some grand ideas for arq, but they will have to wait until later in the year

@samuelcolvin samuelcolvin changed the title Unmaintained? New library SAQ New library SAQ Jan 26, 2022
@sondrelg
Copy link
Contributor

Kudos for communicating this @samuelcolvin 👏

Assuming @tobymao accepts the role as contributor, would porting the UI from SAQ be something you'd be interested in? It's a feature I'd love to see added personally.

@tobymao
Copy link
Contributor Author

tobymao commented Jan 27, 2022

hey @samuelcolvin,

becoming a contributor would be interesting but I have some concerns about how effective I would be since I've migrated my internal project to SAQ.

I would consider it if you were willing to work with me on moving many of the core concepts of SAQ into ARQ -- happy to do my best to keep whatever compatibility you're looking for. Of course I'd respect you as the BDFL, but I'd want to iron out what you're willing to move on or not.

Regarding the pub / sub for results, SAQ does this already. Happy to add it into ARQ.

Regarding your comment about "unfixable" -- I don't think this is true as SAQ does this. Instead of BLPOP though, you should use either BLMOVE or BRPOPLPUSH, this makes dequeuing transactional and there's no worry about jobs disappearing. It also drastically reduces the latency of starting a job -- which is a blocker for me to going back to ARQ.

Are you in a discord or some other chat app so I can chat with you on these details?

@samuelcolvin
Copy link
Member

Really happy to add the UI, but should it be a separate project or a part of arq? I can see advantages to both.

I'm not on discord but I am on telegram. Other option would be a video call to chat it though next week?

@JonasKs
Copy link
Collaborator

JonasKs commented Feb 12, 2022

Just out of curiosity- did you guys discuss anything? 😊

@samuelcolvin
Copy link
Member

I haven't heard anything, sorry if I've missed it. Email is probably the easiest way to get in touch with my initially, [email protected]

@tobymao
Copy link
Contributor Author

tobymao commented Feb 12, 2022

I sent an email to the gmail linked in your git profile 2 weeks ago but i guess you don't check that one, i'll try this one

@tobymao
Copy link
Contributor Author

tobymao commented Feb 14, 2022

i haven't heard back, and i know a couple of people seem to be waiting on this thread, so i guess for now i will decline the offer and continue to work on SAQ as i've just implemented cron jobs.

i understand you're busy and am open to discussing collaboration whenever you're available.

@samuelcolvin
Copy link
Member

Just found your email in spam. I've replied.

@tobymao
Copy link
Contributor Author

tobymao commented Feb 14, 2022

sounds good, i'll message you on telegram

@sondrelg
Copy link
Contributor

Any updates on the plan going forward wrt. maintainership?

@tobymao
Copy link
Contributor Author

tobymao commented Feb 23, 2022

we chatted on telegram, nothing is finalized, no decisions have been made. but it seems like there is a possibility that there could be a collaboration between saq and arq. we will chat again when @samuelcolvin's work settle's down.

@JonasKs
Copy link
Collaborator

JonasKs commented Feb 25, 2022

Thanks for getting back to us Toby. We’re a bit torn on arq at the moment. We want it and like it so much, but not sure how long we can justify using our fork.

@samuelcolvin
Copy link
Member

Hi @JonasKs thanks so much for your patience.

My work has settled down a bit sooner than expect. I should be able to make some progress on arq next week.

@JonasKs
Copy link
Collaborator

JonasKs commented Feb 25, 2022

Oh, nothing to thank for, I appreciate this library and your work a lot! Seriously, you've created multiple libraries I use every day😊 These concerns are purely from a business perspective.

I'm glad to hear that! Let me know if I can help with anything, documentation, tests, running it in QA etc. I'll be available.

@samuelcolvin
Copy link
Member

#437 may be of interest.

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

5 participants