riverqueue / river

Fast and reliable background jobs in Go
https://riverqueue.com
Mozilla Public License 2.0
2.86k stars 68 forks source link

Idempotent queue pause and resume #408

Closed brandur closed 2 weeks ago

brandur commented 2 weeks ago

I was writing tests for River UI and found that when trying to pause an already paused queue or resume an unpaused queue, River returns a "not found" error.

This was quite surprising, so much so that it took me a good half hour of debugging before I considered the fact that it might actually be a problem in upstream River. Even if an argument were to be made that pausing an already paused queue should be an error like "queue already paused", returning "not found" is misleading and guaranteed to result in confused people beyond just myself.

I think it's fine for pause and resume to be considered idempotent operations. If a paused queue is paused again, there's no real damage done, especially if we keep the original paused time, which we do in this implementation.

Similarly, make an update so that when pausing or resuming using the all queues string (*), don't return "not found" error if there are no queues in the database. Instead, just no-op.

brandur commented 2 weeks ago

@bgentry Actually, while looking through that query code I found one other related problem: when you pause/resume using the all queues * string and there are no queues in the database, it returns a "not found" error.

This also feels a bit wrong to me, so I changed it to be a no-op. Do you agree with that behavior? If so, maybe take one more review pass since I added some more code and test cases.

bgentry commented 2 weeks ago

Yes, I agree, a no op seems better in that case!

brandur commented 2 weeks ago

Great, thanks! Also added changelog.