Updated Retries middleware #629
Conversation
It now records requeue_timestamp in message options in before_enqueue function if message failed execution
It now records and asserts all requeue timestamps
dramatiq/middleware/retries.py
Outdated
|
|
||
| # If message will be retried, record in its options time at which it is requeued | ||
| if "retries" in message.options: | ||
| message.options["requeue_timestamp"] = int(time.time() * 1000) |
There was a problem hiding this comment.
Since you’re changing the Retries middleware directly, I would move this line under line 98 and avoid the check and separate before enqueue hook altogether.
There was a problem hiding this comment.
My small concern was that maybe, just maybe, in corner cases there might be some lag between execution of that part of code and enqueue operation, given how dramatiq might be running workers doing heavy CPU operations at the same time.
I shouldn't worry about that?
There was a problem hiding this comment.
I think the most likely place for this thread to relinquish control is going to be the io that is going to be done when actually enqeueuing the message so I think the chances of the suggested approach being less reliable for timing are very slim (it might in fact be more reliable because it’s doing less work).
There was a problem hiding this comment.
Roger that, I'll refactor and push again 🫡
Moved logic for recording requeue_timestamp from before_enqueue to after_process_message
|
Thanks! |
For motivation see discussion at
https://groups.io/g/dramatiq-users/topic/how_to_check_how_long_message/106519634
Retries middleware now records requeue_timestamp on message options in
before_enqueuefunction if message failed execution.Hoped the implementation and tests are up to required standards, if not, let me know what needs to be changed.