digital-fabric / polyphony

Fine-grained concurrency for Ruby
https://www.rubydoc.info/gems/polyphony
MIT License
661 stars 17 forks source link

Undefined method sync for #121

Open dsusviela opened 1 year ago

dsusviela commented 1 year ago

Hello! Thanks for all the hard work being put into the Polyphony gem!

We found an issue when using the gem in some code that interacts with a MySQL database through Sequel.

We have required both polyphony/adapters/sequel and polyphony/adapters/mysql. The error is the following:

An error ocurred inside the call_db function - undefined method `sync' for 

#<Polyphony::FiberConnectionPool:0x000000010bd22178 
  @db=#<Sequel::Mysql2::Database: "mysql2://localhost:3306/a_test_database" {:encoding=>"utf8mb4", :username=>"root"}>, 
  @use_old_connect_api=false,
  @after_connect=nil,
  @connect_sqls=nil,
  @error_classes=[Mysql2::Error],
  @pool=#<Polyphony::ResourcePool:0x000000010bd22128
  @allocator=#<Proc:0x000000010bbadb80 /Users/username/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/polyphony-1.6/lib/polyphony/adapters/sequel.rb:18>,
  @limit=4,
  @size=1,
  @stock=#<Polyphony::Queue:0x000000010bbadb58>,
  @acquired_resources={}>
>

When looking at the stack trace of the error;

"/Users/username/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/sequel-5.65.0/lib/sequel/extensions/connection_validator.rb:65:in `block in extended'"
 "/Users/username/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/sequel-5.65.0/lib/sequel/extensions/connection_validator.rb:64:in `instance_exec'"
 "/Users/username/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/sequel-5.65.0/lib/sequel/extensions/connection_validator.rb:64:in `extended'"
 "/Users/username/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/sequel-5.65.0/lib/sequel/extensions/connection_validator.rb:121:in `extend'"
 "/Users/username/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/sequel-5.65.0/lib/sequel/extensions/connection_validator.rb:121:in `block in <module:Sequel>'"
 "/Users/username/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/sequel-5.65.0/lib/sequel/database/misc.rb:236:in `block in extension'"
 "/Users/username/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/sequel-5.65.0/lib/sequel/database/misc.rb:233:in `each'"
 "/Users/username/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/sequel-5.65.0/lib/sequel/database/misc.rb:233:in `extension'"

We can see that the connection_validator from Squel is making the following call;

    def self.extended(pool)
      pool.instance_exec do
        sync do
          @connection_timestamps ||= {}
          @connection_validation_timeout ||= 3600
        end
      end

      # Make sure the valid connection SQL query is precached,
      # otherwise it's possible it will happen at runtime. While
      # it should work correctly at runtime, it's better to avoid
      # the possibility of failure altogether.
      pool.db.send(:valid_connection_sql)
    end

Where pool is an instance of Polyphony::FiberConnectionPool. The problem is that this class does not implement the sync method and fails. The original sync is just doing a yield inside a mutex, so we did a simple patch on this class by adding the method as

def sync
  yield
end

And it seemed to solve the issue alright. Since we don't really know the implications of this, we were hesitant to open a PR.

Some environment details:

polyphony version: 1.6 ruby version: ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin21] Uname Darwin 21.6.0 Darwin Kernel Version 21.6.0: Mon Aug 22 20:20:05 PDT 2022; root:xnu-8020.140.49~2/RELEASE_ARM64_T8101 arm64