malthe / pq

A PostgreSQL job queueing system
376 stars 41 forks source link

get() timeout not honoured #53

Open jobh opened 4 years ago

jobh commented 4 years ago

The timeout parameter to get() is sometimes sticky, sometimes not honoured.

Consider this sample:

# [...]
import logging; logging.getLogger('pq').setLevel(logging.DEBUG)
queues = pq.PQ(pool=pool)

q = queues['empty_queue_1']
q.get(timeout=5)
q.get(timeout=0)

q = queues['empty_queue_2']
q.get(timeout=0)
q.get(timeout=5)

Expected Behavior

The requested timeouts are honoured, i.e.

Actual Behavior

Instead, the first timeout is used in all subsequent calls, although never less than 1 second.

2020-07-03 16:22:43.095 DEBUG [pq] timeout (5.000 seconds).
2020-07-03 16:22:48.154 DEBUG [pq] timeout (5.000 seconds).
2020-07-03 16:22:49.211 DEBUG [pq] timeout (1.000 seconds).
2020-07-03 16:22:50.271 DEBUG [pq] timeout (1.000 seconds).

Steps to Reproduce the Problem

1. 2. 3.

Specifications

stas commented 4 years ago

Hi there @jobh

Could you share more context to help us understand the test-case a bit. For example, do you have any jobs in the queue?

The way timeout works changed in https://github.com/malthe/pq/commit/362681e6268d9bf09d8d98f1d00368b9f8bb45ca and the goal there is to dynamically pick a shorter timeout if there's a scheduled job. In what case, the pq would skip timeouts if there's immediate work to do.

Now looking at your example:

q.get(timeout=0)

Will always default to the default timeout, since

>>> 0 or 1
1

So, this should be either fixed by checking for Nones or document that 0 is an invalid timeout. The call timeout, also sets the new queue instance timeout, which is probably the confusing part.

Let me know if this answers your questions, or please ask away anything that you don't find clear. I'll be happy to help.

In the meantime, let's wait on merging #55 until we discuss exactly what's the goal with this issue and the PR itself.

You might be interested in taking a look at the query that actually returns the dynamic timeout value: https://github.com/malthe/pq/blob/master/pq/__init__.py#L327-L329