rails / solid_queue

Database-backed Active Job backend
MIT License
1.95k stars 130 forks source link

Running `rails db:migrate` clears out `queue_schema.rb`. #329

Closed gregschmit closed 2 months ago

gregschmit commented 2 months ago

I am trying to install latest solid queue into a project I'm working on, but when I run rails db:migrate then it clears out the queue_schema.rb file. I can load the schema with rails db:prepare, however I do have to run migrations in my primary database, but it always clears out the queue schema. Is there a migration file or multiple migration files for solid queue anymore?

gregschmit commented 2 months ago

I guess I should just take the schema and dump it into a migration file. Sorry if this is a noobie question but I haven't used multiple DBs in Rails before so I'm trying to figure out if my configuration is broken or if this is expected behavior.

gregschmit commented 2 months ago

Maybe if this is the normal current behavior solid queue install should put a migration into the solid_queue_migrate path? Because it does say to set the database configuration migrations_paths to queue_migrate, so I think that would indicate something should be there.

rosa commented 2 months ago

Huh, sorry for the trouble, @gregschmit! That seems really strange! It should just work without you having to do anything, and it shouldn't clear the queue schema. Would you mind sharing your database.yml configuration?

gregschmit commented 2 months ago

Sure, here is my database.yml:

default: &sqlite
  adapter: sqlite3
  pool: <%= ENV.fetch("RAILS_MAX_THREADS", 10) %>
  timeout: 5000

development:
  primary:
    <<: *sqlite
    database: db/development.sqlite3
  solid_queue:
    <<: *sqlite
    database: db/development_queue.sqlite3
    migrations_paths: db/solid_queue_migrate

test:
  primary:
    <<: *sqlite
    database: db/test.sqlite3
  solid_queue:
    <<: *sqlite
    database: db/test_queue.sqlite3
    migrations_paths: db/solid_queue_migrate

production:
  primary:
    <<: *sqlite
    database: db/production.sqlite3
  solid_queue:
    <<: *sqlite
    database: db/production_queue.sqlite3
    migrations_paths: db/solid_queue_migrate

In hindsight to me it makes perfect sense that the schema would be cleared, because there are no migration files inside db/solid_queue_migrate. By the way, I originally had everything set to queue_migrate and queue_schema.rb because that was the default but ended up prepending solid_ just for consistency. I was able to make everything work by putting the template queue_schema.rb into a migration file inside the db/queue_migrate directory and now things are smooth.

gregschmit commented 2 months ago

By the way, it is an open source project I'm using solid queue in, and the latest action passed: https://github.com/gregschmit/rails-rest-framework/actions/runs/10742991994/job/29796849564. You can view the changes I made in the commit https://github.com/gregschmit/rails-rest-framework/commit/bec4b481f25053306f3847e7c5c8bf3c04d1c5d4.

gregschmit commented 2 months ago

I guess my main question is why solid queue doesn't install migrations even in a separate directory, and why the bin/rails solid_queue:install:migrations was removed (and not replaced by rails railties:install:migrations FROM=solid_queue). It seems as long as there are no migrations, then anytime someone runs migrate the schema would be cleared if they don't have an existing local database. The user would have to remember to load the schema first for that particular database.

eduardinni commented 2 months ago

Huh, sorry for the trouble, @gregschmit! That seems really strange! It should just work without you having to do anything, and it shouldn't clear the queue schema. Would you mind sharing your database.yml configuration?

For me it didn't work automatically because I'm using config.active_record.schema_format = :sql.

I've converted the template schema into a migration and now it works, but I'm worried about future changes on the schema, I'll keep the generated queue_schema.rb and check the diffs. Are the future updates planned to land as a migration in the queue's migrations_paths?

thanks for the work on this amazing gem 💎

rosa commented 2 months ago

Yes, these are great points @gregschmit and @eduardinni. We had planned to include future changes to the schema as regular migrations in the queue db migration paths, yes, but you both bring up a good point... let me think a bit more about this.

Thank you 🙏

scratchoo commented 2 months ago

the bin/rails solid_queue:install command created a queue_schema.rb file but with rails db:preparethe database was never created

what I did is:

namespace :solid_queue do
  desc "Load SolidQueue schema into the queue database"
  task load_schema: :environment do
    config = ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, name: "queue")

    ActiveRecord::Base.establish_connection(config)

    load(Rails.root.join("db/queue_schema.rb"))

    puts "SolidQueue schema loaded successfully."
  end
end

This created the database from the solid queue schema file

Finally don't forget to add config.active_job.queue_adapter = :solid_queue to your environment file or to application.rb if you are using solid queue in all environments

I might not be that sophisticated, but it was confusing for me that the install generator was creating a schema file instead of migration file (probably better under a "queue_migrate" folder) and it was not loaded into the database without that rake task!

rosa commented 2 months ago

the bin/rails solid_queue:install command created a queue_schema.rb file but with rails db:prepare the database was never created

Is it possible you didn't have the right configuration in database.yml before running db:prepare? 🤔

dhh commented 2 months ago

@scratchoo I just replicated your account by creating a new app, running solid_queue:install, and the running db:prepare. It doesn't create the database because it hasn't been configured in config/database.yml. Once I added the configuration there (see the top section under # Installation), then it created the database as expected on db:migrate.

@gregschmit You've found a multi-db handling bug in Rails. I've opened https://github.com/rails/rails/issues/52829 to track it. Thanks for tickling that! We fixed a related issue in db:migrate, but db:migrate must be taking a different path. We'll get it sorted before Rails 8.0.0.final. Maybe we can even backport the fix to 7.2.x.

scratchoo commented 2 months ago

@dhh still not working for me,

I have this on database.yml

development:
  primary:
    <<: *default
    database: demo_dev
  queue:
    <<: *default
    database: demo_dev_queue
    migrations_paths: db/queue_migrate

When I run rails db:prepare the output is:

Created database 'demo_dev' Created database 'demo_dev_queue' Created database 'demo_test'

My own migrations seem to run without terminal output (when using db:prepare) but there is no migrations files related to solid_queue that was created, even if I create the folder db/queue_migrate myself, there is no chance!

Supposing that the solid_queues migration files shouldn't be created and only queue_schema.rb should be enough, still the solid_queue tables are never created from that!

My Rails version is 7.1.3.4 in case I am missing something

rosa commented 2 months ago

Supposing that the solid_queues migration files shouldn't be created and only queue_schema.rb should be enough, still the solid_queue tables are never created from that!

Yes, the queue_schema.rb file should be enough, no migration files needs to be created. This is very strange. How are you checking that the tables aren't there? Just by starting solid queue and getting an error? Or in a different way?

scratchoo commented 2 months ago

@rosa by running bin/dev I am getting a bunch of errors type: ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR: relation "solid_queue_processes" does not exist (ActiveRecord::StatementInvalid) etc and eventually the server crashes and don't get started!

on my Procfile.dev I have solidQueueWorker: bundle exec rake solid_queue:start

rosa commented 2 months ago

Thank you! Another question: do you have config.solid_queue.connects_to correctly set for development? In application.rb?

gregschmit commented 2 months ago

Yeah that was an important thing I did was to add the connects_to configuration: https://github.com/gregschmit/rails-rest-framework/blob/72f5871709ffd5a588eea34796eaa096798713f2/test/config/application.rb#L64

I think solid queue should install a migration into queue_migrations directory, not a queue_schema.rb file, because otherwise in the future it causes more work when there are schema changes. You could theoretically ship both, but the schema derives from migrations anyway so I don't see the purpose. Perhaps the thinking is that the solid queue DB can always be dropped because it's not persistent data, but I would still prefer to have migrations installed rather than schema.

scratchoo commented 2 months ago

I didn't but even after adding config.solid_queue.connects_to :my_queue_db_name it didn't solved the issue!

I created a new demo app, and got the same error, I upload it to a repo here: https://github.com/scratchoo/solid_demo

you can use bin/jobs because on this one I don't have a Procfile like with my app

UPDATE:

OOps! thanks to @gregschmit I added the correct values: config.solid_queue.connects_to = {database: {writing: :queue, reading: :queue}} and it's working. the name/symbol of the database should not be the queue database name itself but the key under which the database is categorized. But in any case this wasn't clear and should probably be put on the top within the "Installation" section.

rosa commented 2 months ago

Ahh, looks like that connects_to is wrong. It should look like this:

config.solid_queue.connects_to = { database: { writing: :queue } }
scratchoo commented 2 months ago

@rosa yes I updated my comment above, but still not an obvious thing to do! because of the key database: I would expect to put the queue database name (so how I do even know!) and if this line is important, as I said above, it should be in the installation section.

So yeah, that was the missing part, and it seems that activerecord won't know about the queue database without it.

Thank you.

dhh commented 2 months ago

All this is preconfigured out of the box for new Rails 8 apps, but yes, it's probably worth stressing more forcefully in the installation section of the README that all these have to match. database.yml db name has to correspond to the connects_to name. This is one area where having the database name for connects_to in the solid_queue.yml config might make things a bit easier. But also maybe just more forceful docs will help. I'll take a swing at an update.

rosa commented 2 months ago

Yes, the installer adds that for you but in production.rb. Solid Queue is more intended for production, Rails ships with another adapter for development, the async adapter.

scratchoo commented 2 months ago

@rosa I see that it was added to production.rb by the generator, can I ask why it's only used for writing { database: { writing: :queue } } what about reads?

dhh commented 2 months ago

I've add added the point about other environments and the importance of matching the configuration names for the databases here: https://github.com/rails/solid_queue/commit/dab31d9fbe5d9fde708761aeec6ff75305b66053

dhh commented 2 months ago

@scratchoo If you only set writing, it'll use the same for reading. You only need to add reading if it differs.

rosa commented 2 months ago

I'm going to close this one as there don't seem to be any more specific Solid Queue issues here. Thanks everyone!

gregschmit commented 2 months ago

Totally fine with this closing out, but I just wanted to reiterate my question: why would this gem not release the schema for the separate DB as one initial migration? So if it changes in the future a migration can be added and it would execute when running the routine db:migrate that happens on deploy. It seems like if I upgrade in the future and there are schema changes, I'll have to drop the old DB and re-execute db:prepare, but that wouldn't happen until I noticed the problem.

gregschmit commented 2 months ago

Does db:prepare detect if schema changed and if there are no migration files then it drops the DB and re-executes the schema?

rosa commented 2 months ago

It seems like if I upgrade in the future and there are schema changes, I'll have to drop the old DB and re-execute db:prepare, but that wouldn't happen until I noticed the problem.

No, no, if we add a migration in the future, we'll include it as a migration, and you'll be able to install it and run it with db:migrate.

Migrations are from moving from one schema version to another, not about loading an entire DB, which is what applications setting up Solid Queue for the first time will be doing.

gregschmit commented 2 months ago

Ah, I understand. To be honest it still strikes me as odd to have a "starting schema" rather than an "initial migration" but I'll get over it, especially since it seems to be a pattern that is being embraced by the framework.

gregogalante commented 2 months ago

I'm getting exactly the same problem. I think starting with migrations instead of a pre-generated schema should be the better solution. The solid_cache gem is doing it, so only solid_queue has a different way to work.

rosa commented 2 months ago

Hey @gregogalante, the bug in Rails that came up here was just fixed yesterday: https://github.com/rails/rails/pull/52830

The solid_cache gem is doing it, so only solid_queue has a different way to work.

No, Solid Cache works in the same way as Solid Queue, a schema file that gets copied during install.

gregogalante commented 2 months ago

I just used an old version of solid_cache sorry :D Ok I will try the fix! Thanks for the great work!!

motdotla commented 2 months ago

I think starting with migrations instead of a pre-generated schema should be the better solution.

To be honest it still strikes me as odd to have a "starting schema" rather than an "initial migration"

For me it didn't work automatically because I'm using config.active_record.schema_format = :sql.

came here to +1 all of this. I really didn't like the initial starting schema.

  1. partially because it broke here in a very surprising way
  2. db/migrate_path is such an established standard for decades now, why not just use it
  3. any future migrations necessary to be added to solid_queue will have to go into a db/migrate_path anyways

feels really 2 patterns that could be squashed into one already established one. (i've never been a big db:prepare fan because of edge cases i've ran into. db:migrate has always been reliable)

great to see this is patched here though going forward: https://github.com/rails/rails/pull/52830

thanks for all your hard work! solid_queue is great! just my 2 cents.

jmarsh24 commented 2 months ago

I was able to create a temporary work around by creating generating a migration under db/queue_migrate/xxxxxx_create_initial_solid_queue_tables.rb. This way it will run is a normal migration and not empty the schema file.

motdotla commented 1 month ago

I was able to create a temporary work around by creating generating a migration

yeah, i opted for the same.