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

Sequel + Celluloid + jtds error in 9.1.x #3945

Closed
hkauffman opened this Issue May 31, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@hkauffman

hkauffman commented May 31, 2016

Environment

jruby-9.1.0.0
jruby-9.1.1.0
jruby-9.1.2.0

celluloid 0.17.3
sequel 4.34.0
jdbc-jtds 1.3.1

Issue:

When using jdbc-jtds inside of an actor in a celluloid pool i get the following sequel error thrown from multiple actors:

Sequel::AdapterNotFound: Could not load jdbc/jtds adapter: adapter class not registered in ADAPTER_MAP

Same gem versions + same ruby code work without error in jruby-1.7.x and jruby-9.0 (up to and including 9.0.5.0). No issues if i run the code serially (without celluloid).

Simplified example:

#!/usr/bin/env ruby

require "celluloid/current"
require "sequel"

class Counter
  include Celluloid

  def poll(address)
    db =  Sequel.connect("jdbc:jtds:sybase://#{address}/database", :user => "user", :password => "password")
    cnt = db["select count(*) as cnt from table"].get(:cnt)
    puts "#{address},#{cnt}"
  end
end

devs = ["10.0.1.1", "10.0.1.2", "10.0.1.3", "10.0.1.4", "10.0.1.5"]

pool = Counter.pool

devs.each { |dev| pool.future.poll(dev) }
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jun 9, 2016

Member

@jeremyevans Can you give me a hint why we might be seeing this, before I dig deep into Sequel?

Member

headius commented Jun 9, 2016

@jeremyevans Can you give me a hint why we might be seeing this, before I dig deep into Sequel?

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Jun 9, 2016

Member

@headius wasn't there a loading issue recently? ... think it might happen due concurrent (double) loading.

Member

kares commented Jun 9, 2016

@headius wasn't there a loading issue recently? ... think it might happen due concurrent (double) loading.

@jeremyevans

This comment has been minimized.

Show comment
Hide comment
@jeremyevans

jeremyevans Jun 9, 2016

Contributor

@headius The only thing I can think of is require returning before the file is finished loading (maybe because it is concurrently being loaded by a different thread?).

In terms of what's happening, there is a hash of setup procs (DATABASE_SETUP in sequel/adapters/jdbc.rb). Requiring the adapter file adds an entry to that hash (protected by a mutex). The code to load the adapter checks the hash, and if the entry isn't present (read protected by a mutex), requires the file, and then checks the hash again (read also protected by a mutex). So the only thing that makes sense to me is require returning before the code in the file has been executed.

Contributor

jeremyevans commented Jun 9, 2016

@headius The only thing I can think of is require returning before the file is finished loading (maybe because it is concurrently being loaded by a different thread?).

In terms of what's happening, there is a hash of setup procs (DATABASE_SETUP in sequel/adapters/jdbc.rb). Requiring the adapter file adds an entry to that hash (protected by a mutex). The code to load the adapter checks the hash, and if the entry isn't present (read protected by a mutex), requires the file, and then checks the hash again (read also protected by a mutex). So the only thing that makes sense to me is require returning before the code in the file has been executed.

@chuckremes

This comment has been minimized.

Show comment
Hide comment
@chuckremes

chuckremes Jun 10, 2016

FYI, I ran into this same issue when running the repro script in issue #3398 today. It has 4 threads that all call Sequel.connect using a jdbc connection string and it failed to load that adapter. My workaround was to have the main thread do a call to Sequel.connect against a dummy db in /tmp just to get the driver loaded so the threaded calls wouldn't fail.

chuckremes commented Jun 10, 2016

FYI, I ran into this same issue when running the repro script in issue #3398 today. It has 4 threads that all call Sequel.connect using a jdbc connection string and it failed to load that adapter. My workaround was to have the main thread do a call to Sequel.connect against a dummy db in /tmp just to get the driver loaded so the threaded calls wouldn't fail.

@headius headius added this to the JRuby 9.1.3.0 milestone Jun 11, 2016

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Aug 22, 2016

Member

@jeremyevans I think your theory was correct.

I fixed #4091 in 93707ca, which reverted a bad change made some months ago that prevented the require lock from blocking.

Given the similarity between that issue and this one, I'm going to optimistically call it solved.

Member

headius commented Aug 22, 2016

@jeremyevans I think your theory was correct.

I fixed #4091 in 93707ca, which reverted a bad change made some months ago that prevented the require lock from blocking.

Given the similarity between that issue and this one, I'm going to optimistically call it solved.

@headius headius closed this Aug 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment