embulk / embulk-output-jdbc

MySQL, PostgreSQL, Redshift and generic JDBC output plugins for Embulk
Other
88 stars 86 forks source link

Introduce connection_timeout and socket_timeout to sqlserver #258

Closed hieudion closed 5 years ago

hieudion commented 5 years ago

Currently I meet some trouble after submits the insert query to the server, the job waits for the response but somehow can’t get it and wait forever.

It can be considered as a bug of jTDS lib as mentioned here: https://sourceforge.net/p/jtds/discussion/104389/thread/c3979289/ https://sourceforge.net/p/jtds/bugs/520/ (closed but not resolved)

I think we should introduce timeout for this case.

How to test this: Let's say we start a new transaction and lock the same table when the job is running. It will wait there

`BEGIN TRANSACTION

SELECT * FROM ableTest_0000016c24cdd32d_embulk000 WITH (TABLOCKX, HOLDLOCK)

WHERE 0 = 1

WAITFOR DELAY '02:00'

ROLLBACK TRANSACTION`

hito4t commented 5 years ago

Thank you for the PR!

connectTimeout should be loginTimeout because the JDBC property name in SQL Server JDBC driver is loginTimeout. MySQL JDBC driver and PostgreSQL JDBC driver have connectTimeout property, but they mean the timeout value for a socket connection, not for a database connection.

And I think timeout options shouldn't have default values for compatibility. Namely, if timeout options were not set in config, the plugin shouldn't set their corresponding JDBC properties.

hito4t commented 5 years ago

@hieudion

connectTimeout should be loginTimeout because the JDBC property name in SQL Server JDBC driver is loginTimeout.

I'm sorry, embulk-output-jdbc has connectTimeout property. We should follow it.

hieudion commented 5 years ago

@hito4t I see in the input we already set the loginTimeout in the input: https://github.com/embulk/embulk-input-jdbc/blob/master/embulk-input-sqlserver/src/main/java/org/embulk/input/SQLServerInputPlugin.java#L187 So should we follow it?

For the default values, I will remove it.