ruby-shoryuken / shoryuken

A super efficient Amazon SQS thread based message processor for Ruby. This project is in MAINTENANCE MODE. Reach me out on Slack link on the description if you want to become a new maintainer.
Other
2.06k stars 280 forks source link

FIFO queues and `retry_on` seem incompatible #731

Closed qnm closed 1 year ago

qnm commented 1 year ago

I recently changed a Job to use a FIFO queue. We use FIFO queues already, so changing the queue name was all that was needed to be done.

Later, I noticed an increase in errors relating to SQS.

2023-03-27T06:44:20Z service/workers/d3e06304290e4583ba3bacddf42f59ff E,
[2023-03-27T06:44:20.323041 #11] ERROR -- : [ActiveJob] Failed enqueuing ReportEventBuilderJob
to Shoryuken(production-digivizer-app-default.fifo): Aws::SQS::Errors::InvalidParameterValue
(Value 3 for parameter DelaySeconds is invalid. Reason: The request include parameter that is
not valid for this queue type.)

After much debugging, it seems that using ActiveJobs retry_on with FIFO isn't obviously possible, due to DelaySeconds being not allowed per-message with a FIFO queue[0].

In Rails 6.1.7.3 there is a default wait time of 3 seconds https://github.com/rails/rails/blob/v6.1.7.3/activejob/lib/active_job/exceptions.rb#L56

This is recognised by Rails and routes the call to enqueue_at https://github.com/rails/rails/blob/v6.1.7.3/activejob/lib/active_job/enqueuing.rb#L56

Which in turn is rejected by SQS due to the addition of DelaySeconds in the message. https://github.com/ruby-shoryuken/shoryuken/blob/f94f0707c1acc8ddb772b5fc98787141a68462c8/lib/shoryuken/extensions/active_job_adapter.rb#L45

I'm really not sure where to take this - it feels like it's dangerous to use enqueue_at with a FIFO queue and it should be prevented.

I am happy to work on a PR if anyone has guidance on a) if this is a good idea and b) how I would do it. My naive implementation broke the de-duplication code so looking for a little guidance first!

[0] https://docs.aws.amazon.com/AWSSimpleQueueService/latest/APIReference/API_SendMessage.html

github-actions[bot] commented 1 year ago

This issue is now marked as stale because it hasn't seen activity for a while. Add a comment or it will be closed soon.

github-actions[bot] commented 1 year ago

This issue was closed because it hasn't seen activity for a while.