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

Set RAILS_MAX_THREADS as the default number of worker threads #449

Conversation

genya0407
Copy link
Contributor

The number of worker threads in solid_queue must be less than or equal to the size of the database connection pool.
Otherwise, the worker threads might get stuck while trying to obtain a connection from the pool.

Rails sets RAILS_MAX_THREADS as the default size of the database connection pool:

Therefore, we should set RAILS_MAX_THREADS as the default number of threads for solid_queue.

If the RAILS_MAX_THREADS environment variable is not set, 3 will be a good default value because puma.rb also uses 3 as the default number of worker threads.

Copy link
Member

@rosa rosa left a comment

Choose a reason for hiding this comment

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

Good call! Thank you so much @genya0407! 🙏

@rosa
Copy link
Member

rosa commented Dec 9, 2024

Ah, no, wait! That would cause an error, very probably, because solid queue processes need more DB connections than just the number of threads. They run another thread for the heartbeat, and the polling happens in the main thread.

@epugh
Copy link

epugh commented Dec 9, 2024

Could this be why I am getting this error message:

Error while trying to deserialize arguments: User 'l8k9q6oadrglgzij' has exceeded the 'max_user_connections' resource (current value: 30) 

Because I only have 30 connections available to my MySQL database?

@genya0407
Copy link
Contributor Author

@rosa
Thanks for your feedback!

solid queue processes need more DB connections than just the number of threads.
They run another thread for the heartbeat, and the polling happens in the main thread.

Let me sort out my understanding of Rails/SolidQueue's connection pools.

  • "default" connection pools (every Rails app has these ones: primary / replica, and so on)
  • "queue" connection pool (required by SolidQueue)

Each primary / replica / queue connection pool holds RAILS_MAX_THREADS connections.

Therefore, the following conditions must be satisfied:

  • the_number_of_worker_thread <= (primary|replica)_connection_pool_size
    • This will be satisfied by this PR
  • the_number_of_worker_thread + 1 <= queue_connection_pool_size
    • + 1 is for the polling connection

To satisfy the latter condition, we might need to instruct users to set pool for queue in README.md.
For example:

production:
  primary: &primary_production
    <<: *default
    database: app_production
    username: app
    password: <%= ENV["APP_DATABASE_PASSWORD"] %>
  queue:
    <<: *primary_production
    # 1 connection for each worker thread, 1 connection for the main thread
    pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } + 1 %>
    database: app_production_queue
    migrations_paths: db/queue_migrate

Or we can decrease the number of worker threads like:

  workers:
    - queues: "*"
      threads: <%= ENV.fetch("RAILS_MAX_THREADS", 3) - 1 %> # 1 connection is consumed for polling
      processes: <%%= ENV.fetch("JOB_CONCURRENCY", 1) %>
      polling_interval: 0.1

I believe the former option is better.

@genya0407
Copy link
Contributor Author

@epugh
I don't believe this PR necessarily fixes your error.
While adjusting the thread number in queue.yml might fix this, I feel that max_user_connections = 30 is a very low value, especially if your MySQL server is shared between the application server and the job queue worker.

Increasing the max_user_connections value for your MySQL server should work.

@rosa
Copy link
Member

rosa commented Dec 10, 2024

Thanks @genya0407! It'd be

the_number_of_worker_thread + 2 <= queue_connection_pool_size

with +2 accounting for the polling in the main thread and the heartbeat thread.

The first option you propose (with +2 instead of +1) sounds good, but it'd require changes in Rails to generate the default database.yml.

I'm also thinking a bit more about this, and perhaps it's not desirable to use the same value for Puma threads as for Solid Queue threads since the nature of work there is very different. The fact that the DB pool is set to that size doesn't mean Solid Queue needs to max out the number of connections 🤔

@genya0407
Copy link
Contributor Author

@rosa
If we consider RAILS_MAX_THREADS as a given constraint, perhaps decreasing the number of worker threads would be more natural.
In other words, Rails guarantees that each process can launch RAILS_MAX_THREADS threads, so SolidQueue should adhere to that guarantee.

queue.yml would then look like this:

  workers:
    - queues: "*"
      # 1 connection is consumed for polling, 1 connection is consumed for heartbeat
      threads: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } - 2 %>
      processes: <%%= ENV.fetch("JOB_CONCURRENCY", 1) %>
      polling_interval: 0.1

The fact that the DB pool is set to that size doesn't mean Solid Queue needs to max out the number of connections 🤔

We might consider offering more modest default threads like:

threads: <%= [ENV.fetch("RAILS_MAX_THREADS") { 5 } - 2, 3].min %>

With this, we will be able to avoid stacking in obtaining connections and launching too many worker threads.

@rosa
Copy link
Member

rosa commented Dec 10, 2024

threads: <%= [ENV.fetch("RAILS_MAX_THREADS") { 5 } - 2, 3].min %>

In this case, you need to handle the case where someone has set RAILS_MAX_THREADS to 2 or 1 🤔

In other words, Rails guarantees that each process can launch RAILS_MAX_THREADS threads, so SolidQueue should adhere to that guarantee.

I don't think this is a guarantee or something that necessarily applies to Solid Queue. Rails guarantees that the web server won't run out of DB connections with that setting because it uses that setting for the web server, but it wasn't something set for Solid Queue or even relevant 🤔

I think it'd be simpler to just keep the default of 3, which is documented, and leave it up to the user to decide whether they want it to be the same as their web server threads or not, and adjust DB pool size accordingly if they need to.

@genya0407
Copy link
Contributor Author

I think it'd be simpler to just keep the default of 3, which is documented, and leave it up to the user to decide whether they want it to be the same as their web server threads or not, and adjust DB pool size accordingly if they need to.

I agree. Then I'm closing this PR.

with +2 accounting for the polling in the main thread and the heartbeat thread.

It might be better to document this in README.md as a hint for choosing optimal configuration.
(or I have overlooked the documentation?)

If so, I will open a separate PR later.

@rosa
Copy link
Member

rosa commented Dec 10, 2024

@genya0407 agree! The only documentation we have on threads is in this section. It could include a hint about the DB connection pool. I'd like to include also a warning if the connection pool is too low, as part of starting the supervisor, but that's a different thing that I'll do separately.

@rosa rosa closed this Dec 11, 2024
@genya0407 genya0407 deleted the feature/use-rails-max-threads-as-default-threads-config branch December 11, 2024 13:10
@genya0407
Copy link
Contributor Author

@rosa
Thanks for your helpful discussion 🙏
I have created a new PR: #452

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants