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

em_http_request 1.1.1 causes undefined method(@transaction not defined) #169

Open
bhgames opened this issue Sep 30, 2013 · 14 comments
Open

Comments

@bhgames
Copy link

bhgames commented Sep 30, 2013

Hey,

When I did a bundle update, it force updated my em_http_request 1.1.0 to 1.1.1, and I started to see this error whenever I created or saved any active record object:

NoMethodError: undefined method []' for nil:NilClass from /home/jprince/.rvm/gems/ruby-1.9.3-p429/bundler/gems/em-synchrony-d775d5638cd1/lib/em-synchrony/activerecord.rb:71:incurrent_transaction'

I checked the code and it looks as if @transaction is nil.

Is there anything else you need? Just make an active record object using em_http_request 1.1.1 + em-synchrony.

@igrigorik
Copy link
Owner

Hmm, are you sure you're not pulling in master?

Have a suspicion that this is due to #168. /cc @jonigata

@bhgames
Copy link
Author

bhgames commented Sep 30, 2013

Should I pull in via the PR in #168 ? @igrigorik

@igrigorik
Copy link
Owner

No, the other way around.. I think you're pulling master branch, not last release version (hence "em-synchrony-d775d5638cd1/" in your path). And I think commit in #168 is causing the problem.

@bhgames
Copy link
Author

bhgames commented Sep 30, 2013

Yeah, I switched my :ref to an earlier commit and it went back to working.

On Mon, Sep 30, 2013 at 4:06 PM, Ilya Grigorik [email protected]:

No, the other way around.. I think you're pulling master branch, not last
release version (hence "em-synchrony-d775d5638cd1/" in your path). And I
think commit in #168 https://github.com/igrigorik/em-synchrony/pull/168is causing the problem.


Reply to this email directly or view it on GitHubhttps://github.com//issues/169#issuecomment-25406280
.

@jonigata
Copy link
Contributor

jonigata commented Oct 1, 2013

Hmm, My approach doesn't seem to be good. What should I do...

@jonigata
Copy link
Contributor

jonigata commented Oct 1, 2013

Why 'reset_transaction' had not been called?

If I should do just same behavior as old, I would correct 'current_transaction' as it returns nil when '@transaction' is nil.
But it looks inappropriate to me.

@igrigorik
Copy link
Owner

@bhgames could you isolate the failure into a sample snippet / test case? That would go a long way to resolving this.

@bhgames
Copy link
Author

bhgames commented Oct 1, 2013

Ilya, will try to get to it today. Got a lot on my plate here at work, but
if I have some down time I'll try to create a test project!

On Mon, Sep 30, 2013 at 9:54 PM, Ilya Grigorik [email protected]:

@bhgames https://github.com/bhgames could you isolate the failure into
a sample snippet / test case? That would go a long way to resolving this.


Reply to this email directly or view it on GitHubhttps://github.com//issues/169#issuecomment-25422525
.

@mateuszdw
Copy link
Contributor

Hi, running bundle exec rspec ./spec/activerecord_spec.rb with activerecord = 3.2.8 throw this error

Failures:

  1) Fiberized ActiveRecord driver for mysql2 should fire sequential, synchronous requests within single fiber
     Failure/Error: ActiveRecord::Base.establish_connection(
     RuntimeError:
       invalid binding to detach
     # ./lib/em-synchrony/connection_pool.rb:81:in `block in method_missing'
     # ./lib/em-synchrony/activerecord.rb:120:in `execute'
     # ./lib/em-synchrony/connection_pool.rb:80:in `method_missing'
     # ./spec/activerecord_spec.rb:20:in `establish_connection'
     # ./spec/activerecord_spec.rb:42:in `block (3 levels) in <top (required)>'
     # ./lib/em-synchrony.rb:38:in `block (2 levels) in synchrony'

  2) Fiberized ActiveRecord driver for mysql2 should fire 100 requests in fibers
     Failure/Error: widget = Widget.create title: 'hi'
     NoMethodError:
       undefined method `[]' for nil:NilClass
     # ./lib/em-synchrony/activerecord.rb:71:in `current_transaction'
     # ./lib/em-synchrony/activerecord.rb:98:in `add_transaction_record'
     # ./lib/em-synchrony/activerecord.rb:50:in `block in transaction'
     # ./lib/em-synchrony/activerecord.rb:120:in `execute'
     # ./lib/em-synchrony/activerecord.rb:49:in `transaction'
     # ./spec/activerecord_spec.rb:61:in `block (4 levels) in <top (required)>'
     # ./lib/em-synchrony/fiber_iterator.rb:10:in `call'
     # ./lib/em-synchrony/fiber_iterator.rb:10:in `block (2 levels) in each'

  3) Fiberized ActiveRecord driver for mysql2 should create widget
     Failure/Error: ActiveRecord::Base.establish_connection(
     RuntimeError:
       invalid binding to detach
     # ./lib/em-synchrony/connection_pool.rb:81:in `block in method_missing'
     # ./lib/em-synchrony/activerecord.rb:120:in `execute'
     # ./lib/em-synchrony/connection_pool.rb:80:in `method_missing'
     # ./spec/activerecord_spec.rb:20:in `establish_connection'
     # ./spec/activerecord_spec.rb:70:in `block (3 levels) in <top (required)>'
     # ./lib/em-synchrony.rb:38:in `block (2 levels) in synchrony'

  4) Fiberized ActiveRecord driver for mysql2 should update widget
     Failure/Error: widget = Widget.create title: 'hi'
     NoMethodError:
       undefined method `[]' for nil:NilClass
     # ./lib/em-synchrony/activerecord.rb:71:in `current_transaction'
     # ./lib/em-synchrony/activerecord.rb:98:in `add_transaction_record'
     # ./lib/em-synchrony/activerecord.rb:50:in `block in transaction'
     # ./lib/em-synchrony/activerecord.rb:120:in `execute'
     # ./lib/em-synchrony/activerecord.rb:49:in `transaction'
     # ./spec/activerecord_spec.rb:82:in `block (3 levels) in <top (required)>'
     # ./lib/em-synchrony.rb:38:in `block (2 levels) in synchrony'

  5) Fiberized ActiveRecord driver for mysql2 transactions should work properly
     Failure/Error: ActiveRecord::Base.establish_connection(
     RuntimeError:
       invalid binding to detach
     # ./lib/em-synchrony/connection_pool.rb:81:in `block in method_missing'
     # ./lib/em-synchrony/activerecord.rb:120:in `execute'
     # ./lib/em-synchrony/connection_pool.rb:80:in `method_missing'
     # ./spec/activerecord_spec.rb:20:in `establish_connection'
     # ./spec/activerecord_spec.rb:92:in `block (4 levels) in <top (required)>'
     # ./lib/em-synchrony.rb:38:in `block (2 levels) in synchrony'

on activerecord = 4.0.1

Failures:

  1) Fiberized ActiveRecord driver for mysql2 should fire sequential, synchronous requests within single fiber
     Failure/Error: ActiveRecord::Base.establish_connection(
     RuntimeError:
       invalid binding to detach
     # ./lib/em-synchrony/connection_pool.rb:81:in `block in method_missing'
     # ./lib/em-synchrony/activerecord.rb:120:in `execute'
     # ./lib/em-synchrony/connection_pool.rb:80:in `method_missing'
     # ./spec/activerecord_spec.rb:20:in `establish_connection'
     # ./spec/activerecord_spec.rb:42:in `block (3 levels) in <top (required)>'
     # ./lib/em-synchrony.rb:38:in `block (2 levels) in synchrony'

  2) Fiberized ActiveRecord driver for mysql2 should create widget
     Failure/Error: ActiveRecord::Base.establish_connection(
     RuntimeError:
       invalid binding to detach
     # ./lib/em-synchrony/connection_pool.rb:81:in `block in method_missing'
     # ./lib/em-synchrony/activerecord.rb:120:in `execute'
     # ./lib/em-synchrony/connection_pool.rb:80:in `method_missing'
     # ./spec/activerecord_spec.rb:20:in `establish_connection'
     # ./spec/activerecord_spec.rb:70:in `block (3 levels) in <top (required)>'
     # ./lib/em-synchrony.rb:38:in `block (2 levels) in synchrony'

  3) Fiberized ActiveRecord driver for mysql2 transactions should work properly
     Failure/Error: ActiveRecord::Base.establish_connection(
     RuntimeError:
       invalid binding to detach
     # ./lib/em-synchrony/connection_pool.rb:81:in `block in method_missing'
     # ./lib/em-synchrony/activerecord.rb:120:in `execute'
     # ./lib/em-synchrony/connection_pool.rb:80:in `method_missing'
     # ./spec/activerecord_spec.rb:20:in `establish_connection'
     # ./spec/activerecord_spec.rb:92:in `block (4 levels) in <top (required)>'
     # ./lib/em-synchrony.rb:38:in `block (2 levels) in synchrony'

maybe it helps someone smart to solve this issue
Thanks

@temochka
Copy link

Since #168 is closed, I’ll post my comment here. The introduced fix seems to be incompatible with ActiveRecord versions prior to 4.0 because it makes a use of the recently added @transaction instance variable.

I was surprised to find out that the driver seemed to never work properly, since all fibers share same transaction state (and any other state managed by EMMysqlAdapter) making it impossible to process simultaneous transactions opened in multiple fibers. I pulled the master hoping that #168 fixed the issue, but it wasn’t much help, because the project I’m working on is using ActiveRecord 3.2.16.

The whole approach of sharing a single adapter with a constant pool of MySQL connections across multiple fibers seems a bit odd to me. This way the driver becomes too sensitive to race conditions and changes in ActiveRecord. Ideally every fiber needs an instance of ActiveRecord::ConnectionAdapters::EMMysql2Adapter, so we don’t have to perform peculiar state manipulations like ones in #168. I understand that it is probably the only way to do it without too much ActiveRecord patching, though I’m wondering if somebody with a better understanding of both ActiveRecord and Synchrony could come up with a better approach.

Theoretically it is possible to have a synchrony-aware ConnectionHandler which instantiates a ActiveRecord::ConnectionAdapters::ConnectionPool-compatible connection adapters pool. Though I expect problems caused by usage of thread attributes in ActiveRecord::Base if we decide to go this way.

@dmgtn
Copy link
Contributor

dmgtn commented Feb 9, 2014

We have better implementation of em_mysql2 adapter with fiber-aware connection pooling.

But it is not tested yet with ActiveRecord 4.0+.

@dmgtn
Copy link
Contributor

dmgtn commented Feb 9, 2014

Pull request: #179 .

@dgutov
Copy link
Collaborator

dgutov commented Jun 2, 2014

I've been seeing a similar problem (invalid binding to detach errors) with our test suite when trying to upgrade mysql2 from 0.3.11. Apparently, it's the latest released version that doesn't trigger the problem. All more recent ones do, at least with em-synchrony 1.0.3.

@dgutov
Copy link
Collaborator

dgutov commented Mar 12, 2015

While 1.0.4 plainly refuses to work with ActiveRecord 3.2, 1.0.3 seems to work, more or less, but spuriously fails the transactions example (about 50% of the time on my machine), no matter which version of mysql2 is used:

Failures:

  1) Fiberized ActiveRecord driver for mysql2 transactions should work properly
     Failure/Error: widget.title.should eq('hello')

       expected: "hello"
            got: "hi14"

       (compared using ==)
     # ./spec/activerecord_spec.rb:103:in `block (5 levels) in <top (required)>'
     # ./spec/activerecord_spec.rb:102:in `each'
     # ./spec/activerecord_spec.rb:102:in `block (4 levels) in <top (required)>'
     # ./lib/em-synchrony.rb:38:in `block (2 levels) in synchrony'

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

No branches or pull requests

7 participants