Skip to content

MONGOID-5882 Isolation state #6009

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

jamis
Copy link
Contributor

@jamis jamis commented Jul 8, 2025

Some applications will use threads for concurrency. Others will use fibers. Prior to this PR, Mongoid worked fine with threads, but it's internal state could get into odd states when run under fibers.

This PR allows applications to indicate which isolation level they wish to use, and Mongoid's state will be isolated to that scope.

# the default -- inherit the isolation level from `ActiveSupport::IsolatedExecutionState` if possible
Mongoid.isolation_level = :rails

# The following two explicit settings are supported:
Mongoid.isolation_level = :thread # the classic behavior
Mongoid.isolation_level = :fiber  # NEW!

When using the :fiber isolation level, Mongoid's internal state will be inherited from any parent fiber. If you want to make sure a fiber begins with a clean slate, you can wipe the isolation state with Mongoid::Threaded.reset!.

@johnnyshields
Copy link
Contributor

Does :fiber cause weird side effects with embedded callbacks?

@jamis
Copy link
Contributor Author

jamis commented Jul 10, 2025

Does :fiber cause weird side effects with embedded callbacks?

It shouldn't. That's what most of the tests in this PR are designed to ensure: that nested fibers inherit state from their parent fiber. The entire point of this PR is to make sure Mongoid can be compatible with fiber-based libraries like Falcon.

@johnnyshields
Copy link
Contributor

johnnyshields commented Jul 10, 2025

Cool. 2 more:

  1. Is it correct to assume that "fiber" isolation also implies "thread" isolation, as fibers exist on threads (and the "main fiber" of each thread is a separate fiber?) Are there tests for this?
  2. Aside from not being on the latest Ruby, is there any reason that a user using Puma (not Falcon) wouldn't want to use :fiber isolation? If no (i.e. fiber is safe) then how about making it the default in Mongoid 10?

@travisbell
Copy link

travisbell commented Jul 10, 2025

Awesome, thanks @jamis!

Only one comment from me, does it make sense to default the value of Mongoid.isolation_level to the value of ActiveSupport::IsolatedExecutionState.isolation_level? I believe ActiveRecord does, so for continuity, it kind of feels like maybe Mongoid should as well? This way I don't have to remember to set it twice.

P.S. Falcon also assumes this is the standard entry point to make such decisions: https://github.com/socketry/falcon/blob/main/lib/falcon/railtie.rb

@jamis
Copy link
Contributor Author

jamis commented Jul 10, 2025

@travisbell -- great suggestion. I'll look into it. It's a bit complicated by the fact that we still support Rails 6.x, and the IsolatedExecutionState wasn't introduced until sometime in 7.x, but it's a good idea.

@jamis
Copy link
Contributor Author

jamis commented Jul 10, 2025

@johnnyshields :

  1. Is it correct to assume that "fiber" isolation also implies "thread" isolation, as fibers exist on threads (and the "main fiber" of each thread is a separate fiber?) Are there tests for this?

See https://github.com/mongodb/mongoid/pull/6009/files#diff-fc964a96de10aff651e8e1d4a35745a2a33f357819002b11af3cdacfe3e56979R58-R78

  1. Aside from not being on the latest Ruby, is there any reason that a user using Puma (not Falcon) wouldn't want to use :fiber isolation? If no (i.e. fiber is safe) then how about making it the default in Mongoid 10?

It's worth considering. Again, "aside from not being on the latest Ruby" is the sticking point. As long as we support Ruby prior to 3.2, we really can't make :fiber the default.

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