search

timgit / pg-boss

Queueing jobs in Postgres from Node.js like a boss
MIT License
2.07k stars 157 forks source link

fix: limit 1 active singleton queue job #368

Closed adamhamlin closed 1 year ago

adamhamlin commented 1 year ago

See relevant issue: https://github.com/timgit/pg-boss/issues/367

This PR would change the behavior of "singleton queue" jobs (i.e., singletonKey + useSingletonQueue=true) as follows:

My assumption is that this is a bug--or at least not intentional or desired behavior--and so it does not warrant a breaking change or the need for configurability.

coveralls commented 1 year ago

Coverage Status

Coverage: 100.0%. Remained the same when pulling c11d12746a622939d41bb91ead14984d43bf7cac on adamhamlin:fix-singleton-queue into 9938b1a03bd1ff979282d549958ce382cc7a06e4 on timgit:master.

adamhamlin commented 1 year ago

Hmmm, 1 of the travis jobs passed, but the other 2 failed (on different tests, tho both singleton-related). Locally, I get no failures. Any ideas @timgit? resolved

phips28 commented 1 year ago

I tried something similar in the past, read here why there might be „race conditions“ when 2 workers arrive at the same time. https://github.com/timgit/pg-boss/issues/274#issuecomment-932771001

adamhamlin commented 1 year ago

I tried something similar in the past, read here why there might be „race conditions“ when 2 workers arrive at the same time. #274 (comment)

I could be missing something, but that comment talks about 2 concurrent workers selecting two different queued jobs, but for singleton queue there can only be 1 queued job to begin with.

phips28 commented 1 year ago

right, than this might work 😄 I have not worked with those queues yet. I just recalled my old issue where I tried to do something similar (1 concurrent job per queue across multiple workers) - so I thought I just throw it in 😉

adamhamlin commented 1 year ago

right, than this might work 😄 I have not worked with those queues yet. I just recalled my old issue where I tried to do something similar (1 concurrent job per queue across multiple workers) - so I thought I just throw it in 😉

I appreciate it @phips28! Thank you

adamhamlin commented 1 year ago

@timgit just confirming you're not waiting on me for anything

adamhamlin commented 1 year ago

@timgit sorry for the spam, just hoping to move this toward resolution