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

Add enqueue_after_transaction_commit? for Rails 7.2 compatibility #777

Open jpawlyn opened 2 months ago

jpawlyn commented 2 months ago

For Rails 7.2 there is a new method enqueue_after_transaction_commit? required on an ActiveJob adapter class - see:

https://github.com/rails/rails/blob/dfd1e951aa1aeef06c39fffb2994db8a8fa1914f/activejob/lib/active_job/queue_adapters/abstract_adapter.rb#L10-L16 and https://api.rubyonrails.org/classes/ActiveJob/QueueAdapters/AbstractAdapter.html#method-i-enqueue_after_transaction_commit-3F

jpawlyn commented 2 months ago

Just realised that the problem with inheriting from ActiveJob::QueueAdapters::AbstractAdapter is that the test will only pass with active job 7.2 but not sure how to do update the test to be conditional on the active job version.

Alternatively we could just use the initial commit or even remove the test. Any advice appreciated.

jpawlyn commented 1 month ago

Just realised that the problem with inheriting from ActiveJob::QueueAdapters::AbstractAdapter is that the test will only pass with active job 7.2 but not sure how to do update the test to be conditional on the active job version.

Alternatively we could just use the initial commit or even remove the test. Any advice appreciated.

I suspect removing the test probably makes for the simplest solution. Let me do that.

FinnIckler commented 1 month ago

It looks like ActiveJob might be changing the way it is handling enqueue_after_transaction_commit https://github.com/rails/rails/pull/53336 and https://github.com/rails/rails/issues/52675 So this might be unnecessary soon. Still think this would be nice to have in the interim

jeropaul commented 4 weeks ago

hmm reading through https://github.com/rails/rails/pull/53375/files and I'm a little stumped on the recommended path for Shoryuken users.

For rails 8 will not be an issue. For rails 7.2 should be set the configuration never?

    Rails.application.config.active_job.enqueue_after_transaction_commit = :never
jpawlyn commented 4 weeks ago

hmm reading through https://github.com/rails/rails/pull/53375/files and I'm a little stumped on the recommended path for Shoryuken users.

For rails 8 will not be an issue. For rails 7.2 should be set the configuration never?

    Rails.application.config.active_job.enqueue_after_transaction_commit = :never

By inheriting from ActiveJob::QueueAdapters::AbstractAdapter we should be good I think for Rails 7.2.

phstc commented 3 weeks ago

@jpawlyn I can no longer maintain this project. I updated the README with additional info https://github.com/ruby-shoryuken/shoryuken/commit/946ed1d9bb211c8b3b88cb911df48f9229ba8f45

januszm commented 3 weeks ago

@jpawlyn I can no longer maintain this project.

Might be a good moment to transition to AWS Rails SDK that adds their own maintained SQS adapter for ActiveJob. I already tested in one app in production and works fine, although it's a slightly different approach, where jobs are executed in a running Rails app, not a separate worker process.

https://github.com/aws/aws-sdk-rails

Gemfile

gem 'aws-sdk-rails', '~> 4'

config/aws_sqs_active_job.yml

queues:
  default: "https://sqs.us-east-2.amazonaws.com/ACCOUNT_ID/QUEUE_NAME-<%= Rails.env %>-default.fifo"

config/environments/production.rb

config.active_job.queue_adapter = :sqs
januszm commented 3 weeks ago

@jpawlyn instead of inheriting, you could maybe include ActiveJob::Enqueuing if it makes things simpler?

januszm commented 2 weeks ago

@jeropaul as a temporary solution before Rails 8, you can monkey patch Shoryuken with this in the shoryuken initializer file for example (config/initializers/shoryuken.rb or wherever you put your initialization code)

module ActiveJob
  module QueueAdapters
    class ShoryukenAdapter
      def enqueue_after_transaction_commit?
        false
      end
    end
  end
end
SteffanPerry commented 2 weeks ago

@jpawlyn I can no longer maintain this project.

Might be a good moment to transition to AWS Rails SDK that adds their own maintained SQS adapter for ActiveJob. I already tested in one app in production and works fine, although it's a slightly different approach, where jobs are executed in a running Rails app, not a separate worker process.

https://github.com/aws/aws-sdk-rails

I have been using the aws gem for well over a year "aws rails sdk". It does have some downside that this gem doesn't. mainly around running jobs on a worker outside of AWS Lambda. In our situation we have an event based application with many queues, one per worker. The AWS gem forces us to choose between, severly over provisioning the workers, or underutilizing the workers. This is because you can not setup a config where you want say 20 job max concurrently running. you have to choose how many queues essentially are running on the machine as a process will only run one queue at a time and hoping they all dont peak at the same time. This can lead to accidently overloading your machine in peak times, or just wasting $$ on resources if you over provision.

januszm commented 2 weeks ago

@SteffanPerry , true, it's a different architecture. It may evolve towards replacing shoryuken but for now it's not a 1-1 drop-in replacement, I also still stick to Shoryuken in production.