Ensure isolation_level is set.

[ioquatix]

To avoid potential mis-configuration when used with rails, we can use a railtie to set the isolation_level correctly.

See #218 for more context.

I don't like how we are (optionally) coupling falcon to Rails, but I'm not sure there is a good alternative. I considered a separate gem, but people may miss it.

Types of Changes

• New feature.

Contribution

• I added tests for my changes.
• I tested my changes locally.
• I agree to the Developer's Certificate of Origin 1.1.

[trevorturk]

This change makes me a bit nervous. I attempted to enable :fiber in my Rails app and it created a huge number of connections and overwhelmed my database. I may be misunderstanding what's going on here, but I've been collecting some links for further research:

• Fiber-safe ConnectionPool rails/rails#44219
• Fiber safe ActiveRecord connection pool  rails/rails#42271 (comment)
• Isolation level and Active Record Connection Pool falcon-rails-example#2
• Use case for limiting the number of fibers generated. #161
• PoC: Add an option to disable connection checkout caching rails/rails#50793

I think the issue is that Rails will check out a connection when needed, but keep the connection open for the duration of the request. So, if you're hanging on a long running HTTP request (as I am) before sending a response, you may have a lot of open connections at once. There appears to be a workaround in surrounding database calls with a with_connection block, and some of the recent Rails work seems like it may improve the situation since a connection might be released earlier, thus mitigating (but not fundamentally solving) the issue, since I believe you'd still effectively have an uncapped number of connections.

The alternative (using :thread) results in only have a single connection per Falcon worker, which is obviously not ideal, but I'm not sure what a better alternative is at this time (setting aside the with_connection workaround.) I was thinking we might need some way to keep the connection pool in the top level fiber? (I think Falcon has a top level fiber that all child process are descended from?)

In any case, I'm happy to discuss further, and apologies if I'm misunderstanding things here, but I wanted to call attention because I think this may be something that a user should make a conscious decision to enable vs being automatic?

[ioquatix]

Thanks for raising your concern, and it's very reasonable.

I believe you can still explicitly override the isolation level IIUC, so you can explicitly opt into per-thread connection handling/isolation in your config. Do you mind confirming this works? i.e. add an explicit config in config/application.rb and then check ActiveSupport::IsolatedExecutionState.isolation_level.

If that turns out to be a valid use case (which it sounds like it is in your case), let's add further documentation to clarify it.

I believe most users will want this behaviour (per-fiber by default), and the next step is to provide a per-query AR connection pool mode. Once we have that, we can probably ALSO add that here, e.g. config.active_record.connection_pool.persist = false or something.

There are several people working on the AR connection pool issues so I believe we will get to a proper solution by Rails 7.2 if we are lucky or 8.0 if the changes are more extensive.

[trevorturk]

Thanks for the thoughtful reply.

After giving it some more consideration, I agree isolation_level = :fiber seems like a better default when using Rails with Falcon from what I'm seeing in GitHub discussions etc, so I'm feeling better about this change. I've been watching the activity around per-query AR connection pooling, which is very exciting for the future!

I was able to confirm that you can override the default in config/application.rb if needed, so that's a clear exit ramp for anyone that runs into problems today. I'm still a bit worried that someone may upgrade Falcon without noticing this change, but I'd imagine that won't be a huge number of people given the current landscape, so it should be fine. It seems like more people are running into the inverse issue, thus changing the default makes sense.

As for considering a bit more documentation, I was thinking perhaps something like the following change/addition:

Rails 7.1 introduced the ability to change its internal isolation level from threads (default) to fibers. When you use falcon with Rails, it will automatically set the isolation level to fibers.

...becomes:

Rails 7.1 introduced the ability to change its internal isolation level from threads (default) to fibers. When you use falcon with Rails, it will automatically set the isolation level to fibers. Beware that this change may increase the utilization of shared resources such as Active Record's connection pool. If you'd like to retain the default behavior, add the following to config/application.rb to reset the isolation level to threads: config.active_support.isolation_level = :thread.

Let me know if you'd like me to draft a PR, or feel free to use that as a starting point, or ignore this if you think it's not worth documenting at this point.

Thanks again!

[ioquatix]

That looks super, please draft a PR :)