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

Seed database with inline ActiveJob job adapter #34953

Merged
merged 1 commit into from Jan 17, 2019

Conversation

@gmcgibbon
Copy link
Member

@gmcgibbon gmcgibbon commented Jan 17, 2019

Summary

Closes #34939.

It appears that ActiveStorage enqueues ActiveStorage::AnalyzeJobs when attaching a file. This prompts ActiveJob to enqueue jobs (via the async queue adapter by default in development) and can cause problems for one-off scripts such as database seeding.

This may also be a symptom of a bigger problem with ActiveStorage's queueing assumptions, or maybe the async job adapter. For now, I think temporarily setting the queue adapter to inline for all seeding in all environments is reasonable.

@rails-bot rails-bot bot added the railties label Jan 17, 2019
railties/test/railties/engine_test.rb Outdated

boot_rails

assert_equal ActiveJob::QueueAdapters::InlineAdapter, Rails.application.load_seed

This comment has been minimized.

@rafaelfranca

rafaelfranca Jan 17, 2019
Member

I think this doesn't do what you want. Rails.application.load_seed will return true.

@gmcgibbon gmcgibbon force-pushed the gmcgibbon:seed_with_inline_jobs branch Jan 17, 2019
@gmcgibbon gmcgibbon force-pushed the gmcgibbon:seed_with_inline_jobs branch to 66cc0e7 Jan 17, 2019
@rafaelfranca rafaelfranca merged commit 2dee59f into rails:master Jan 17, 2019
2 checks passed
2 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@gmcgibbon gmcgibbon deleted the gmcgibbon:seed_with_inline_jobs branch Jan 17, 2019
rafaelfranca added a commit that referenced this pull request Jan 17, 2019
Seed database with inline ActiveJob job adapter
@olivierlacan
Copy link
Contributor

@olivierlacan olivierlacan commented Jun 7, 2019

This caused us a myriad of problems and I wish it were:

  • better documented
  • easy to disable and surfaced as a configuration in config/environmnets/development.rb

We have auto-publication code that pushes things out to Kafka or RabbitMQ when records are saved. We want to be able to use this in development because it's harmless (it just queues a job with Delayed::Job) but when seeding, this change causes all these jobs to be executed inline. This results in HTTP errors that confused me enough that I almost disabled job queueing in development.

It took spying on ActiveJob's queue_adapter= method to noticed that it was being called three times:

That's a lot of surprising behavior and a lot of places to search to find it. If anything, setting queue_adapter in an environment should override this logic (the :inline adapter) and warn that jobs queued in the seeds will not be performed now. Does that make sense?

@olivierlacan
Copy link
Contributor

@olivierlacan olivierlacan commented Jun 7, 2019

As @rafaelfranca pointed out this has been fixed in 5-2-stable by only overriding :async (and no other adapters like :delayed_job) with :inline during seeding. See #35905.

Still feels like it might be confusing, so I'm hoping anyone surprised finds this as a reference later on.

suketa added a commit to suketa/rails_sandbox that referenced this pull request Jul 21, 2019
Seed database with inline ActiveJob job adapter
rails/rails#34953)
jonathanhefner added a commit to jonathanhefner/rails that referenced this pull request Nov 15, 2020
Before rails#34953, when using the `:async` Active Job queue adapter, jobs
enqueued in `db/seeds.rb`, such as Active Storage analysis jobs, would
cause a hang (see rails#34939).  Therefore, rails#34953 changed all jobs enqueued
in `db/seeds.rb` to use the `:inline` queue adapter instead.  (This
behavior was later limited to only take effect when the `:async` adapter
was configured, see rails#35905.)  However, inline jobs in `db/seeds.rb`
cleared `CurrentAttributes` values (see rails#37526).  Therefore, rails#37568
changed the `:inline` adapter to wrap each job in its own thread, for
isolation.  However, wrapping a job in its own thread affects which
database connection it uses.  Thus inline jobs can no longer execute
within the calling thread's database transaction, including seeing any
uncommitted changes.  Additionally, if the calling thread is not wrapped
with the executor, the inline job thread (which is wrapped with the
executor) can deadlock on the load interlock.  And when testing (with
`connection_pool.lock_thread = true`), the inline job thread can
deadlock on one of the locks added by rails#28083.

Therefore, this commit reverts the solutions of rails#34953 and rails#37568, and
instead wraps evaluation of `db/seeds.rb` with the executor.  This
eliminates the original hang from rails#34939, which was also due to running
multiple threads and not wrapping all of them with the executor.  And,
because nested calls to `executor.wrap` are ignored, any inline jobs in
`db/seeds.rb` will not clear `CurrentAttributes` values.

Alternative fix for rails#34939.
Reverts rails#34953.
Reverts rails#35905.
Partially reverts rails#35896.

Alternative fix for rails#37526.
Reverts rails#37568.

Fixes rails#40552.
jonathanhefner added a commit to jonathanhefner/rails that referenced this pull request Nov 18, 2020
Before rails#34953, when using the `:async` Active Job queue adapter, jobs
enqueued in `db/seeds.rb`, such as Active Storage analysis jobs, would
cause a hang (see rails#34939).  Therefore, rails#34953 changed all jobs enqueued
in `db/seeds.rb` to use the `:inline` queue adapter instead.  (This
behavior was later limited to only take effect when the `:async` adapter
was configured, see rails#35905.)  However, inline jobs in `db/seeds.rb`
cleared `CurrentAttributes` values (see rails#37526).  Therefore, rails#37568
changed the `:inline` adapter to wrap each job in its own thread, for
isolation.  However, wrapping a job in its own thread affects which
database connection it uses.  Thus inline jobs can no longer execute
within the calling thread's database transaction, including seeing any
uncommitted changes.  Additionally, if the calling thread is not wrapped
with the executor, the inline job thread (which is wrapped with the
executor) can deadlock on the load interlock.  And when testing (with
`connection_pool.lock_thread = true`), the inline job thread can
deadlock on one of the locks added by rails#28083.

Therefore, this commit reverts the solutions of rails#34953 and rails#37568, and
instead wraps evaluation of `db/seeds.rb` with the executor.  This
eliminates the original hang from rails#34939, which was also due to running
multiple threads and not wrapping all of them with the executor.  And,
because nested calls to `executor.wrap` are ignored, any inline jobs in
`db/seeds.rb` will not clear `CurrentAttributes` values.

Alternative fix for rails#34939.
Reverts rails#34953.
Reverts rails#35905.
Partially reverts rails#35896.

Alternative fix for rails#37526.
Reverts rails#37568.

Fixes rails#40552.
jonathanhefner added a commit to jonathanhefner/rails that referenced this pull request Dec 2, 2020
Before rails#34953, when using the `:async` Active Job queue adapter, jobs
enqueued in `db/seeds.rb`, such as Active Storage analysis jobs, would
cause a hang (see rails#34939).  Therefore, rails#34953 changed all jobs enqueued
in `db/seeds.rb` to use the `:inline` queue adapter instead.  (This
behavior was later limited to only take effect when the `:async` adapter
was configured, see rails#35905.)  However, inline jobs in `db/seeds.rb`
cleared `CurrentAttributes` values (see rails#37526).  Therefore, rails#37568
changed the `:inline` adapter to wrap each job in its own thread, for
isolation.  However, wrapping a job in its own thread affects which
database connection it uses.  Thus inline jobs can no longer execute
within the calling thread's database transaction, including seeing any
uncommitted changes.  Additionally, if the calling thread is not wrapped
with the executor, the inline job thread (which is wrapped with the
executor) can deadlock on the load interlock.  And when testing (with
`connection_pool.lock_thread = true`), the inline job thread can
deadlock on one of the locks added by rails#28083.

Therefore, this commit reverts the solutions of rails#34953 and rails#37568, and
instead wraps evaluation of `db/seeds.rb` with the executor.  This
eliminates the original hang from rails#34939, which was also due to running
multiple threads and not wrapping all of them with the executor.  And,
because nested calls to `executor.wrap` are ignored, any inline jobs in
`db/seeds.rb` will not clear `CurrentAttributes` values.

Alternative fix for rails#34939.
Reverts rails#34953.
Reverts rails#35905.
Partially reverts rails#35896.

Alternative fix for rails#37526.
Reverts rails#37568.

Fixes rails#40552.

(cherry picked from commit 648da12)
wbotelhos added a commit to wbotelhos/rails that referenced this pull request Jul 18, 2021
Before rails#34953, when using the `:async` Active Job queue adapter, jobs
enqueued in `db/seeds.rb`, such as Active Storage analysis jobs, would
cause a hang (see rails#34939).  Therefore, rails#34953 changed all jobs enqueued
in `db/seeds.rb` to use the `:inline` queue adapter instead.  (This
behavior was later limited to only take effect when the `:async` adapter
was configured, see rails#35905.)  However, inline jobs in `db/seeds.rb`
cleared `CurrentAttributes` values (see rails#37526).  Therefore, rails#37568
changed the `:inline` adapter to wrap each job in its own thread, for
isolation.  However, wrapping a job in its own thread affects which
database connection it uses.  Thus inline jobs can no longer execute
within the calling thread's database transaction, including seeing any
uncommitted changes.  Additionally, if the calling thread is not wrapped
with the executor, the inline job thread (which is wrapped with the
executor) can deadlock on the load interlock.  And when testing (with
`connection_pool.lock_thread = true`), the inline job thread can
deadlock on one of the locks added by rails#28083.

Therefore, this commit reverts the solutions of rails#34953 and rails#37568, and
instead wraps evaluation of `db/seeds.rb` with the executor.  This
eliminates the original hang from rails#34939, which was also due to running
multiple threads and not wrapping all of them with the executor.  And,
because nested calls to `executor.wrap` are ignored, any inline jobs in
`db/seeds.rb` will not clear `CurrentAttributes` values.

Alternative fix for rails#34939.
Reverts rails#34953.
Reverts rails#35905.
Partially reverts rails#35896.

Alternative fix for rails#37526.
Reverts rails#37568.

Fixes rails#40552.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants