lbehnke / h2database

Automatically exported from code.google.com/p/h2database
0 stars 0 forks source link

Accordance with javax.sql.CommonDataSource#setLoginTimeout specification #70

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Hello Thomas,

org.h2.jdbcx.JdbcConnectionPool implements javax.sql.DataSource which
extends javax.sql.CommonDataSource and this interface specifies for the
method javax.sql.CommonDataSource#setLoginTimeout()
--------------------------------------------------------------------------------
Sets the maximum time in seconds that this data source will wait while 
attempting to connect to a database. A value of zero specifies that the timeout
is the default system timeout if there is one; otherwise, it specifies that
there is no timeout. When a DataSource object is created, the login timeout is
initially zero.
--------------------------------------------------------------------------------

But calling org.h2.jdbcx.JdbcConnectionPool#setLoginTimeout(0) with a zero
value breaks the pool: a SQL exception is always thrown when trying to get
a connection, which has to be returned in no time at all.

# A simple change at line 181 will do:
if(i >= timeout) => if(timeout > 0 && i > timeout) {

The side effect is negligible, the exception is thrown when the delay is
stricly greater than the defined timeout.

# And maybe at line 134
public synchronized void setLoginTimeout(int seconds) {
    if(seconds == 0) {
        seconds = DriverManager.getLoginTimeout();
    }
    this.timeout = seconds;
}

Keep up the good work, I recently definitely switched from MySQL to H2 as
my primary development database, and I'm promoting it in production
environment.

Cheers,

Olivier

Original issue reported on code.google.com by opar...@gmail.com on 24 Mar 2009 at 1:56

GoogleCodeExporter commented 9 years ago
Hi,

Thanks for the bug report! I will fix this in the next release.

> the default system timeout if there is one

Do you think this refers to DriverManager.getLoginTimeout(), or does it refer 
to the
default for this implementation (which would be 60 seconds)? I'm not sure, I 
think
they mean for this implementation. Would that be OK as well?

Original comment by thomas.t...@gmail.com on 25 Mar 2009 at 8:37

GoogleCodeExporter commented 9 years ago

Original comment by thomas.t...@gmail.com on 25 Mar 2009 at 8:38

GoogleCodeExporter commented 9 years ago
This should be fixed with todays release (1.1.110).

Original comment by thomas.t...@gmail.com on 3 Apr 2009 at 3:59

GoogleCodeExporter commented 9 years ago
Hi Thomas,

I gave a second thought on the whole thing, and refined the algorithm.

Basically, it respects this order:
- JdbcPoolConnector specified timeout, if any
- underlying DataSource timeout, if any
- DriverManager timeout, if any
- JdbcPoolConnector default timeout : set by default to 60s.

The timeout is checked every getConnection() is called.

Alas, the code is on my laptop and the battery is dead, I can't copy it to my 
desktop
this weekend. I'll post it later but it doesn't matter if it's not making it 
for the
next release.

What do you think of this algorithm ?

Olivier

Original comment by opar...@gmail.com on 3 Apr 2009 at 8:36

GoogleCodeExporter commented 9 years ago
Hi,

The algorithm is a bit complicated... Why do you need the timeout at all?

Regards,
Thomas

Original comment by thomas.t...@gmail.com on 5 Apr 2009 at 11:42

GoogleCodeExporter commented 9 years ago
Hi,

My plan is to set the login timeout to 5 minutes by default in the next release.

Original comment by thomas.t...@gmail.com on 27 Apr 2009 at 6:33

GoogleCodeExporter commented 9 years ago
In the latest release the login timeout is 5 minutes.

Original comment by thomas.t...@gmail.com on 1 May 2009 at 3:59