swaldman / c3p0

a mature, highly concurrent JDBC Connection pooling library, with support for caching and reuse of PreparedStatements.
http://www.mchange.com/projects/c3p0
Other
1.28k stars 338 forks source link

No retry when acquireRetryAttempts is set to 0 #132

Closed marcatl closed 6 months ago

marcatl commented 5 years ago

Hi,

According to the documentation, when acquireRetryAttempts is set to 0, c3p0 should try to acquire new connections indefinitely, with a delay of acquireRetryDelay between attempts. I have the following settings: [INFO] (main) com.mchange.v2.c3p0.impl.AbstractPoolBackedDataSource: Initializing c3p0 pool... com.mchange.v2.c3p0.ComboPooledDataSource [ acquireIncrement -> 4, acquireRetryAttempts -> 0, acquireRetryDelay -> 1000, autoCommitOnClose -> false, automaticTestTable -> null, breakAfterAcquireFailure -> false, checkoutTimeout -> 10000, connectionCustomizerClassName -> null, connectionTesterClassName -> com.mchange.v2.c3p0.impl.DefaultConnectionTester, contextClassLoaderSource -> caller, dataSourceName -> , debugUnreturnedConnectionStackTraces -> false, description -> null, driverClass -> org.postgresql.Driver, extensions -> {}, factoryClassLocation -> null, forceIgnoreUnresolvedTransactions -> false, forceSynchronousCheckins -> false, forceUseNamedDriverClass -> false, identityToken -> , idleConnectionTestPeriod -> 60, initialPoolSize -> 1, jdbcUrl -> jdbc:postgresql://**, maxAdministrativeTaskTime -> 0, maxConnectionAge -> 1800, maxIdleTime -> 900, maxIdleTimeExcessConnections -> 120, maxPoolSize -> 10, maxStatements -> 0, maxStatementsPerConnection -> 25, minPoolSize -> 1, numHelperThreads -> 8, preferredTestQuery -> SELECT 1, privilegeSpawnedThreads -> false, properties -> {user=, password=}, propertyCycle -> 0, statementCacheNumDeferredCloseThreads -> 0, testConnectionOnCheckin -> false, testConnectionOnCheckout -> false, unreturnedConnectionTimeout -> 0, userOverrides -> {}, usesTraditionalReflectiveProxies -> false ]

and I get the following:

com.mchange.v2.resourcepool.BasicResourcePool: com.mchange.v2.resourcepool.BasicResourcePool$ScatteredAcquireTask@6a9b0a6f -- Acquisition Attempt Failed!!! Clearing pending acquires. While trying to acquire a needed new resource, we failed to succeed more than the maximum number of allowed acquisition attempts (0). Last acquisition attempt exception: { org.postgresql.util.PSQLException: Connection refused. Check that the hostname and port are correct and that the postmaster is accepting TCP/IP connections....

then

com.mchange.v2.resourcepool.BasicResourcePool: Having failed to acquire a resource, com.mchange.v2.resourcepool.BasicResourcePool@10667848 is interrupting all Threads waiting on a resource to check out. Will try again in response to new client requests.

and then

com.mchange.v2.resourcepool.CannotAcquireResourceException: A ResourcePool could not acquire a resource from its primary factory or source. at com.mchange.v2.resourcepool.BasicResourcePool.awaitAvailable(BasicResourcePool.java:1469) at com.mchange.v2.resourcepool.BasicResourcePool.prelimCheckoutResource(BasicResourcePool.java:644) at com.mchange.v2.resourcepool.BasicResourcePool.checkoutResource(BasicResourcePool.java:554) at com.mchange.v2.c3p0.impl.C3P0PooledConnectionPool.checkoutAndMarkConnectionInUse(C3P0PooledConnectionPool.java:758) at com.mchange.v2.c3p0.impl.C3P0PooledConnectionPool.checkoutPooledConnection(C3P0PooledConnectionPool.java:685)

I would expect to see some retries and to get the last error after 10s (according to the value set for checkoutTimeout), but it's thrown right away. Also, I find it strange that 'BasicResourcePool is interrupting all Threads waiting on a resource to check out.'

If I change acquireRetryAttempts to -1, I do see retries and after 10s I get com.mchange.v2.resourcepool.TimeoutException: A client timed out while waiting to acquire a resource from com.mchange.v2.resourcepool.BasicResourcePool@1d01dfa5 -- timeout at awaitAvailable(). I would expect to have the same behavior when acquireRetryAttempts is set to 0.

MichaelMih commented 5 years ago

Same behavior with my code. Running 0.9.5.4. Looks like AcquireRetryAttempts=0 just disables any retries and fails immediately and AcquireRetryAttempts<0 retries indefinitely. This looks very logical to me, but it goes against the docs that say "If this value is less than or equal to zero, c3p0 will keep trying to fetch a Connection indefinitely"

lnezvalova commented 4 years ago

@swaldman Can we assume this is just mistake in documentation? 0 as disable seems the only way how to disable the retries so this looks really like doc-issue, however we would like to make sure we won't be surprised if this change in future, as this would mean transition from disable to infinite with current configuration.

swaldman commented 6 months ago

The behavior in the documentation is the behavior that was intended, but it indeed seems not to be the behavior that was implemented!

At this point, it's probably best to bring the documentation into sync with the behavior, so users won't be surprised if they upgrade with working current configurations. I'll do that.

swaldman commented 6 months ago

Taking another look, the actual, currently-implemented behavior is probably best in the merits to, the number refers to retries, so zero should mean one try, zero retries, and only negative values are out of band.

The long awaited (or forgotten, or given-up-upon) 0.10.0 release will include the corrected documentation. See https://github.com/swaldman/c3p0/commit/24a5fedb25afbafdc86aafa773ca4c7b6b941b64

Thanks!