Question: Why Limit Dequeue to 1 Message?

[grvhi]

Just curious to know why the MaxNumberOfMessages have been fixed to 1? Would it not make sense to return a batch of messages (SQS max is 10)?

https://github.com/igalarzab/sqjobs/blob/master/sqjobs/connectors/sqs.py#L87

Looks like a nice, simple library though! Thanks!

[igalarzab]

Hi George!

I'm glad you liked it :)

The MaxNumberOfMessages is fixed to 1 to improve the concurrency. If you have more than one worker (something very usual) getting more than one job per request can cause to have a lot of jobs waiting to be run in one worker while the other workers are completely free, waiting. Also, it simplifies the error handling in case that something wrong happens in the execution.

Do you need to increase the number?

Cheers!

[grvhi]

Thanks @igalarzab

I hear what you're saying, but I think it only really applies for low throughput queues wherein the number of messages waiting to be processed is low (i.e. close to or less than the number of workers to consume them).

This is not the case for us, so having multiple workers fetching 10 messages at a time is not going to leave any workers feeling left out! Not only that, but it will reduce the SQS bill (and time between jobs) to fetch multiple messages at once.

On the error handling front, I can see the concern, but if results are cached after each execution, then the bulk versions of delete_message and change_message_visibility (which is already being used) are called after processing each batch, that should allow for simple recovery?

I suppose the need to offer a clean-up method will be a bit more complex (to handle a shutdown request) wherein pending messages will have to be returned to the queue, but I don't think that should be too hard to implement?

Unless I've missed something, I think those are the only issues which need to be catered for? If so, I'll try and put some time into writing solutions over the weekend and submit a PR.

[grvhi]

@igalarzab - I've done a really quick commit (untested) to demonstrate an approach I think might work:

https://github.com/Erve1879/sqjobs/commit/7003755ce525a038b26f9ee2921e5ed7ec3f5a0d

Would be interested to hear your thoughts.

[igalarzab]

Hi @Erve1879,

You are completely right that if there are a lot of enqueued messages getting more than one message at a time it's better. I've seen your commit and it's a great solution.

Just my two cents:

• I think most of the functionality for retrieving just one message could be deleted (1 message is a degenerated case of N-messages), simplifying the code.
• I see you are adding also a way to stop gracefully the worker. It could be great to have another PR with that functionality :)

If you want after creating the PR we can check the changes more carefully, and if you want any help just tell me, please!

[grvhi]

Great @igalarzab - I'll try and clean it up tomorrow.

[patoroco]

Any news @Erve1879 ?