How locking, dequeue and TaskState::InProgress are working together

[kirillsalykin]

Hi,

Could you please help me understand how locking, dequeuing, and TaskState::InProgress are working together now? I'm not sure I fully understand this.

The reason I'm interested is that I implemented things a bit differently:

1. The worker periodically pulls for new tasks with a Pending status, using SKIP LOCKED.
2. If a new task is found, it is fetched using FOR UPDATE (essentially locking it, so it won't appear under SKIP LOCKED).
3. The worker runs the task within the same transaction, and once the task is finished, it is marked as completed or failed.
4. If the worker is interrupted (for any reason), the transaction is rolled back (removing the lock), and the task returns to the Pending state. (even if transaction was not rolled back by the worker - it will be rolled back by psql)
   This approach renders the InProgress state redundant.

The InProgress approach assumes that if a worker is interrupted, the task should be switched back to Pending (so all cases should be handled). I initially used the InProgress approach as well, but because we were using preemptible nodes, the workers were frequently interrupted. Occasionally, due to an unknown issue (maybe a race condition?), some jobs ended up stuck in the InProgress state without any workers running them.

Maybe it's just my bad experience.

There is a downside to my approach as well: if a task runs for a long time, it keeps the transaction open, which is not considered good practice.

What do you think?

[maxcountryman]

That's how underway works today: in fact other transactions never see the "in-progress" state so it's probably redundant.

On that note, there's no doubt a decent amount of optimization that could be done with the queries an especially indices.

[kirillsalykin]

ah, oke
so basically, https://github.com/maxcountryman/underway/blob/main/src/queue.rs#L494 is not needed, right?

[kirillsalykin]

"in-progress" state so it's probably redundant.

that was my initial thought, thanks for confirmaiton.

[kirillsalykin]

Wouldnt you mind If I do my first PR and remove it?

[maxcountryman]

So it's not entirely redundant (I'm remembering my original thought process now): notice how dequeue is implemented?

When I was designing the queue I wanted to fully decouple it from the other parts of the system.

Why did I bother doing that?

Well the idea is that you could implement your own worker (for example you might not want to keep the transaction open for the duration of execute) and that's much easier if we provide a more granular set of states.

But please don't be discouraged from contributing: I just wouldn't want to waste your time with that context in mind. 🙂