Memoization of boolean causes extra calls to SQS

[timbreitkreutz]

This was causing us to get rate limited by SQS. Our team eventually found this boolean memoization bug which solved the rate limiting problem for us.

[timbreitkreutz]

@phstc Thanks for the feedback. I think this is ready to go :)

[phstc]

@timbreitkreutz thanks for the fix 🍻 Shoryuken 3.3.1 is out with your fix.

[phstc]

Hi @timbreitkreutz

I think I found another unexpected behavior with FIFO queues and Shoryuken:

The message group ID is the tag that specifies that a message belongs to a specific message group. Messages that belong to the same message group are always processed one by one, in a strict order relative to the message group (however, messages that belong to different message groups might be processed out of order).

https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/using-messagegroupid-property.html

Messages that belong to the same message group are always processed one by one

In the way Shoryuken works now, it is grabbing up to 10 messages per receive_message even for FIFOs, see https://github.com/phstc/shoryuken/blob/46456fb4aa00321ee95ac40b19e472613d0b93ab/lib/shoryuken/fetcher.rb#L50

What I think it should do instead is every time it fetches messages for a FIFO queue, it should fetch one at the time:

    def receive_messages(queue, limit)
      options = receive_options(queue)

      shoryuken_queue = Shoryuken::Client.queues(queue.name)

      options[:max_number_of_messages]  = shoryuken_queue.fifo? ? 1 : max_number_of_messages(limit, options)
      options[:message_attribute_names] = %w[All]
      options[:attribute_names]         = %w[All]

      options.merge!(queue.options)

      shoryuken_queue.receive_messages(options)
    end

Otherwise, you may end up processing more than one message per group at the time.

WDYT?

[timbreitkreutz]

Makes sense to me— we are not yet using fifo queues with shoryuken though.