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

current tenant is lost when creating a new fiber #181

Open
derikson opened this issue Dec 23, 2021 · 4 comments · May be fixed by #182
Open

current tenant is lost when creating a new fiber #181

derikson opened this issue Dec 23, 2021 · 4 comments · May be fixed by #182

Comments

@derikson
Copy link

Steps to reproduce

  Apartment::Tenant.switch! 'db1'
  puts Apartment::Tenant.current                           # prints "db1"
  Fiber.new { puts Apartment::Tenant.current }.resume      # prints "public"
  puts (1..2).lazy.map { Apartment::Tenant.current }.peek  # prints "public"
  User.limit(2)
      .find_each(batch_size: 1)
      .lazy
      .map { puts Apartment::Tenant.current }.next         # prints "public"
  puts Apartment::Tenant.current                           # prints "db1"

Expected behavior

All of the above should print "db1".

Actual behavior

Inside a new fiber, the fiber local variable :apartment_adapter no longer exists, so it creates a new one, set to the default tenant. Since enumerators are implemented using fibers, some enumerator methods use the default tenant instead of the tenant of the parent fiber.

Apartment::Tenant.adapter was probably meant to use thread local variables, but Ruby 1.9.2 changed the meaning of Thread#[] and Thread#[]= to operate on fiber local variables instead of thread local variables. Instead Thread#thread_variable_get and Thread#thread_variable_set should be used.

System configuration

  • Database: (Tell us what database and its version you use.)
    Postgres 12.6
  • Apartment version:
    2.9.0
  • use_schemas: true
  • Rails (or ActiveRecord) version:
    6.1.4.4
  • Ruby version:
    2.7.5
@IlyaUmanets
Copy link

Hey @derikson, I'm not a contributor but I will tell you my opinion.

I'd say it's expected behavior, I faced the same case on my project and we weren't surprised to see a public tenant under a new thread even though it was not public in a parent.

@rpbaltazar
Copy link
Contributor

Hi @derikson, I don't have a use case for fibers so for me it's harder to make an informed decision. I do have a use case where I switch the DB connection from a writer to a reader db and I have the apartment setting the same tenant on the new connection.
All of this to say, I do understand your concern and maybe we can make it configurable.

@lunks
Copy link
Contributor

lunks commented Mar 2, 2022

I think what @derikson presented here should be considered a bug according to his code. lazy shouldn't cause us to lose the current tenant.

@derikson
Copy link
Author

derikson commented Mar 2, 2022

@rpbaltazar I'm no longer working on a project that uses this gem, so I no longer have a use case that this solves. If nothing else, this bug report might help someone who is getting data from different tenants mixed together track down the cause.

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 a pull request may close this issue.

4 participants