Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add GoodJob as Active Job backend #47

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app-rails/Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,5 @@ group :production do
# Add plugin for pg gem to support AWS RDS IAM
gem "pg-aws_rds_iam", "~> 0.5.0"
end

gem "good_job", "~> 3.29"
14 changes: 14 additions & 0 deletions app-rails/Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ GEM
dotenv (3.1.0)
drb (2.2.1)
erubi (1.12.0)
et-orbi (1.2.11)
tzinfo
factory_bot (6.4.6)
activesupport (>= 5.0.0)
factory_bot_rails (6.4.3)
Expand All @@ -182,8 +184,18 @@ GEM
faraday-net_http (3.1.0)
net-http
ffi (1.16.3)
fugit (1.11.0)
et-orbi (~> 1, >= 1.2.11)
raabro (~> 1.4)
globalid (1.2.1)
activesupport (>= 6.1)
good_job (3.29.5)
activejob (>= 6.0.0)
activerecord (>= 6.0.0)
concurrent-ruby (>= 1.0.2)
fugit (>= 1.1)
railties (>= 6.0.0)
thor (>= 0.14.1)
i18n (1.14.4)
concurrent-ruby (~> 1.0)
importmap-rails (2.0.1)
Expand Down Expand Up @@ -266,6 +278,7 @@ GEM
rspec-expectations (~> 3.12)
rspec-mocks (~> 3.12)
rspec-support (~> 3.12)
raabro (1.4.0)
racc (1.7.3)
rack (3.0.10)
rack-session (2.0.0)
Expand Down Expand Up @@ -467,6 +480,7 @@ DEPENDENCIES
factory_bot_rails
faker (~> 3.2)
faraday
good_job (~> 3.29)
importmap-rails
jbuilder
jwt
Expand Down
4 changes: 2 additions & 2 deletions app-rails/app/services/notification_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ def self.send_email_notification(mailer, mailer_args, mailer_action, recipients
recipients: recipients
})

mail_message.deliver_later

recipients.each do |recipient|
mailer_args[:email_address] = recipient
mail_message = mailer
Expand All @@ -25,8 +27,6 @@ def self.send_email_notification(mailer, mailer_args, mailer_action, recipients
msg: "Generated mail message",
mail_message: mail_message
})

mail_message.deliver_now
end
end
end
5 changes: 4 additions & 1 deletion app-rails/config/environments/development.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,17 @@
# Highlight code that triggered database queries in logs.
config.active_record.verbose_query_logs = true

# Use "GoodJob" for Active Job backend - docs at https://github.com/bensheldon/good_job
config.active_job.queue_adapter = :good_job

# Highlight code that enqueued background job in logs.
config.active_job.verbose_enqueue_logs = true

# Suppress logger output for asset requests.
config.assets.quiet = true

# Allow web_console to render when triggered from the rails app running locally in a docker container.
config.web_console.permissions = ["192.168.0.0/16", "172.16.0.0/16", "10.0.0.0/8"]
config.web_console.permissions = [ "192.168.0.0/16", "172.16.0.0/16", "10.0.0.0/8" ]

# Raises error for missing translations.
# config.i18n.raise_on_missing_translations = true
Expand Down
12 changes: 8 additions & 4 deletions app-rails/config/environments/production.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
# Exclude healthcheck endpoint from force SSL since healthchecks should not go through
# the reverse proxy.
# See https://api.rubyonrails.org/classes/ActionDispatch/SSL.html
config.ssl_options = { redirect: { exclude: -> request { /health/.match?(request.path) } } }
config.ssl_options = { redirect: { exclude: ->(request) { /health/.match?(request.path) } } }

# Log to STDOUT by default
config.logger = ActiveSupport::Logger.new(STDOUT)
Expand All @@ -75,9 +75,13 @@
# Use a different cache store in production.
# config.cache_store = :mem_cache_store

# Use a real queuing backend for Active Job (and separate queues per environment).
# config.active_job.queue_adapter = :resque
# config.active_job.queue_name_prefix = "app_rails_production"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we need a queue_name_prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a lot more Rails expertise than I do so please LMK if this is the wrong line of thinking, but since jobs are queued using a table in the DB I don't think that we'd need to worry about a prefix since we won't be sharing the database across environments? I would think that the queue prefix might be more necessary if we were doing something like sharing a Redis instance across multiple environments, for example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since jobs are queued using a table in the DB I don't think that we'd need to worry about a prefix since we won't be sharing the database across environments?

Good point!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After looking at this and thinking about it more, I think we should move this line:

  config.active_job.queue_adapter = :good_job

out of the per-environment files and into /app-rails/config/application.rb. What do you think? Good Jobs should definitely be used in the mock-production environment. I'm not sure what it might do to the test environment, though. Thoughts?

# Use "GoodJob" for Active Job backend - docs at https://github.com/bensheldon/good_job
# GoodJob was picked as a default because it uses Postgres (already a part of this template)
# and has good adoption and maintenence.
Comment on lines +79 to +80
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should get moved to an ADR and a link to that should be added here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do, thanks!

config.active_job.queue_adapter = :good_job
# This mode of execution runs jobs async in the same thread pool as the Rails server.
# Project teams will want to evaluate the best way to run queued jobs for their project.
ellery-nava marked this conversation as resolved.
Show resolved Hide resolved
config.good_job.execution_mode = :async

config.action_mailer.delivery_method = :sesv2
config.action_mailer.perform_caching = false
Expand Down
101 changes: 101 additions & 0 deletions app-rails/db/migrate/20240627160705_create_good_jobs.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# frozen_string_literal: true

class CreateGoodJobs < ActiveRecord::Migration[7.1]
def change
# Uncomment for Postgres v12 or earlier to enable gen_random_uuid() support
# enable_extension 'pgcrypto'

create_table :good_jobs, id: :uuid do |t|
t.text :queue_name
t.integer :priority
t.jsonb :serialized_params
t.datetime :scheduled_at
t.datetime :performed_at
t.datetime :finished_at
t.text :error

t.timestamps

t.uuid :active_job_id
t.text :concurrency_key
t.text :cron_key
t.uuid :retried_good_job_id
t.datetime :cron_at

t.uuid :batch_id
t.uuid :batch_callback_id

t.boolean :is_discrete
t.integer :executions_count
t.text :job_class
t.integer :error_event, limit: 2
t.text :labels, array: true
t.uuid :locked_by_id
t.datetime :locked_at
end

create_table :good_job_batches, id: :uuid do |t|
t.timestamps
t.text :description
t.jsonb :serialized_properties
t.text :on_finish
t.text :on_success
t.text :on_discard
t.text :callback_queue_name
t.integer :callback_priority
t.datetime :enqueued_at
t.datetime :discarded_at
t.datetime :finished_at
end

create_table :good_job_executions, id: :uuid do |t|
t.timestamps

t.uuid :active_job_id, null: false
t.text :job_class
t.text :queue_name
t.jsonb :serialized_params
t.datetime :scheduled_at
t.datetime :finished_at
t.text :error
t.integer :error_event, limit: 2
t.text :error_backtrace, array: true
t.uuid :process_id
end

create_table :good_job_processes, id: :uuid do |t|
t.timestamps
t.jsonb :state
t.integer :lock_type, limit: 2
end

create_table :good_job_settings, id: :uuid do |t|
t.timestamps
t.text :key
t.jsonb :value
t.index :key, unique: true
end

add_index :good_jobs, :scheduled_at, where: "(finished_at IS NULL)", name: :index_good_jobs_on_scheduled_at
add_index :good_jobs, [ :queue_name, :scheduled_at ], where: "(finished_at IS NULL)", name: :index_good_jobs_on_queue_name_and_scheduled_at
add_index :good_jobs, [ :active_job_id, :created_at ], name: :index_good_jobs_on_active_job_id_and_created_at
add_index :good_jobs, :concurrency_key, where: "(finished_at IS NULL)", name: :index_good_jobs_on_concurrency_key_when_unfinished
add_index :good_jobs, [ :cron_key, :created_at ], where: "(cron_key IS NOT NULL)", name: :index_good_jobs_on_cron_key_and_created_at_cond
add_index :good_jobs, [ :cron_key, :cron_at ], where: "(cron_key IS NOT NULL)", unique: true, name: :index_good_jobs_on_cron_key_and_cron_at_cond
add_index :good_jobs, [ :finished_at ], where: "retried_good_job_id IS NULL AND finished_at IS NOT NULL", name: :index_good_jobs_jobs_on_finished_at
add_index :good_jobs, [ :priority, :created_at ], order: { priority: "DESC NULLS LAST", created_at: :asc },
where: "finished_at IS NULL", name: :index_good_jobs_jobs_on_priority_created_at_when_unfinished
add_index :good_jobs, [ :priority, :created_at ], order: { priority: "ASC NULLS LAST", created_at: :asc },
where: "finished_at IS NULL", name: :index_good_job_jobs_for_candidate_lookup
add_index :good_jobs, [ :batch_id ], where: "batch_id IS NOT NULL"
add_index :good_jobs, [ :batch_callback_id ], where: "batch_callback_id IS NOT NULL"
add_index :good_jobs, :labels, using: :gin, where: "(labels IS NOT NULL)", name: :index_good_jobs_on_labels

add_index :good_job_executions, [ :active_job_id, :created_at ], name: :index_good_job_executions_on_active_job_id_and_created_at
add_index :good_jobs, [ :priority, :scheduled_at ], order: { priority: "ASC NULLS LAST", scheduled_at: :asc },
where: "finished_at IS NULL AND locked_by_id IS NULL", name: :index_good_jobs_on_priority_scheduled_at_unfinished_unlocked
add_index :good_jobs, :locked_by_id,
where: "locked_by_id IS NOT NULL", name: "index_good_jobs_on_locked_by_id"
add_index :good_job_executions, [ :process_id, :created_at ], name: :index_good_job_executions_on_process_id_and_created_at
end
end
89 changes: 88 additions & 1 deletion app-rails/db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading