search

rails-sqlserver / tiny_tds

TinyTDS - Simple and fast FreeTDS bindings for Ruby using DB-Library.
Other
605 stars 191 forks source link

TinyTds::Client.new blocks other threads #421

Open ebeigarts opened 5 years ago

ebeigarts commented 5 years ago

Environment

ProductName: Mac OS X ProductVersion: 10.13.5 BuildVersion: 17F77 TinyTDS Version: 2.1.2 FreeTDS Version: 1.00.104 Ruby: 2.3.1

Description

TinyTds::Client.new blocks the other threads for 60s if the mssql server is not reachable. Am I missing something or this is the expected behavior?

Example:

https://gist.github.com/ebeigarts/85c5d6603f8dab78b04addcff2ef3d0c

ruby 2.3.1
tiny_tds 2.1.2
MAIN: 2018-11-28 13:20:17 +0200
THR1: 2018-11-28 13:20:17 +0200
THR2: 2018-11-28 13:20:17 +0200 START
MAIN: 2018-11-28 13:20:22 +0200
THR2: 2018-11-28 13:20:22 +0200 FAIL Unable to connect: Adaptive Server is unavailable or does not exist (10.193.15.203:1433)
THR1: 2018-11-28 13:20:22 +0200
MAIN: 2018-11-28 13:20:23 +0200
THR1: 2018-11-28 13:20:23 +0200
MAIN: 2018-11-28 13:20:24 +0200
THR1: 2018-11-28 13:20:24 +0200
MAIN: 2018-11-28 13:20:25 +0200
THR1: 2018-11-28 13:20:25 +0200
THR1: 2018-11-28 13:20:26 +0200
MAIN: 2018-11-28 13:20:26 +0200
THR1: 2018-11-28 13:20:27 +0200
MAIN: 2018-11-28 13:20:27 +0200
THR1: 2018-11-28 13:20:28 +0200
MAIN: 2018-11-28 13:20:28 +0200
MAIN: 2018-11-28 13:20:29 +0200
THR1: 2018-11-28 13:20:29 +0200
THR1: 2018-11-28 13:20:30 +0200
MAIN: 2018-11-28 13:20:30 +0200
metaskills commented 5 years ago

I would think expected behavior for Ruby and compared to other DB connection gems. You can lower the connect timeout if you want.

ebeigarts commented 5 years ago

@metaskills we will probably lower the connect_timeout for now, or skip using threads.

At least pg doesn't block - https://gist.github.com/08c1b336952df04f0e4645f376d995dc

ruby 2.3.1
pg
THR2: 2018-11-28 14:56:57 +0200 START
THR1: 2018-11-28 14:56:57 +0200
MAIN: 2018-11-28 14:56:57 +0200
THR1: 2018-11-28 14:56:58 +0200
MAIN: 2018-11-28 14:56:58 +0200
THR1: 2018-11-28 14:56:59 +0200
MAIN: 2018-11-28 14:56:59 +0200
MAIN: 2018-11-28 14:57:00 +0200
THR1: 2018-11-28 14:57:00 +0200
MAIN: 2018-11-28 14:57:01 +0200
THR1: 2018-11-28 14:57:01 +0200
THR2: 2018-11-28 14:57:02 +0200 FAIL timeout expired
THR1: 2018-11-28 14:57:02 +0200
MAIN: 2018-11-28 14:57:02 +0200
THR1: 2018-11-28 14:57:04 +0200
MAIN: 2018-11-28 14:57:04 +0200
MAIN: 2018-11-28 14:57:05 +0200
THR1: 2018-11-28 14:57:05 +0200
THR1: 2018-11-28 14:57:06 +0200
MAIN: 2018-11-28 14:57:06 +0200
MAIN: 2018-11-28 14:57:07 +0200
THR1: 2018-11-28 14:57:07 +0200

And mysql2 also doesn't block - https://gist.github.com/866e58a6fbc3cfda3f2a7268607c49aa

ruby 2.3.1
mysql2
THR1: 2018-11-28 15:05:52 +0200
THR2: 2018-11-28 15:05:52 +0200 START
MAIN: 2018-11-28 15:05:52 +0200
THR1: 2018-11-28 15:05:53 +0200
MAIN: 2018-11-28 15:05:53 +0200
THR1: 2018-11-28 15:05:54 +0200
MAIN: 2018-11-28 15:05:54 +0200
MAIN: 2018-11-28 15:05:55 +0200
THR1: 2018-11-28 15:05:55 +0200
MAIN: 2018-11-28 15:05:56 +0200
THR1: 2018-11-28 15:05:56 +0200
THR2: 2018-11-28 15:05:57 +0200 FAIL Can't connect to MySQL server on '10.193.15.203' (60)
MAIN: 2018-11-28 15:05:57 +0200
THR1: 2018-11-28 15:05:57 +0200
MAIN: 2018-11-28 15:05:58 +0200
THR1: 2018-11-28 15:05:58 +0200
MAIN: 2018-11-28 15:05:59 +0200
THR1: 2018-11-28 15:05:59 +0200
THR1: 2018-11-28 15:06:00 +0200
MAIN: 2018-11-28 15:06:00 +0200
THR1: 2018-11-28 15:06:01 +0200
MAIN: 2018-11-28 15:06:01 +0200
metaskills commented 5 years ago

Cool, if you find that other DB connection gems behave differently, please do let us know so we can make this better.

ebeigarts commented 5 years ago

@metaskills as I mentioned in the previous comment, other db gems do behave differently - they don't block other threads, but tinytds blocks other threads

metaskills commented 5 years ago

My bad, I missed that. I wonder if that is something we can fix at our bindings to FreeTDS. I see we removed at one time the rb_thread_blocking_region and used rb_thread_blocking_region which leads back to this commit (https://github.com/rails-sqlserver/tiny_tds/commit/3eb33c62). Maybe our error handler needs to block? Do you see anything that could be changed?