Commons Dbcp
  1. Commons Dbcp
  2. DBCP-291

setting maxWait does not work as expected

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.3
    • Labels:
      None

      Description

      My expectation was that if maxWait is set to X seconds and there are N requests waiting to get a connection (in a case where the pool is bounded and
      all the connection are used) if none of the used connection gets free than all N requests will time-out at the same time.
      In reality it seems that 1 request will be timed-out after X seconds the second one after 2 * X and the last one after N * X.
      The problem is that getPooledConnectionAndInfo is synchronized (and therefore will look all N requests) and only one request will be processed at the time.
      This process includes waiting until freed connection or timed-out (done by commons.pool GenericKeyedObjectPool).
      I am not sure why the getPooledConnectionAndInfo has to be synchronized and not only the pool creation part.

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        586d 15h 10m 1 Mark Thomas 21/Jun/09 16:27
        Resolved Resolved Reopened Reopened
        185d 7h 11m 1 Phil Steitz 23/Dec/09 23:39
        Reopened Reopened Resolved Resolved
        20h 34m 1 Phil Steitz 24/Dec/09 20:13
        Resolved Resolved Closed Closed
        52d 6h 9m 1 Phil Steitz 15/Feb/10 02:23
        Phil Steitz made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Phil Steitz made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Phil Steitz added a comment -

        Fixed threadsafety issue in r893809

        Show
        Phil Steitz added a comment - Fixed threadsafety issue in r893809
        Phil Steitz made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Phil Steitz added a comment -

        We need to protect the userkeys map. The uderlying LRUMap impl is not threadsafe. I just got this:

        org.apache.commons.dbcp.SQLNestedException: Could not retrieve connection info from pool
        [junit] at org.apache.commons.dbcp.datasources.SharedPoolDataSource.getPooledConnectionAndInfo(SharedPoolDataSource.java:183)
        [junit] at org.apache.commons.dbcp.datasources.InstanceKeyDataSource.getConnection(InstanceKeyDataSource.java:680)
        [junit] at org.apache.commons.dbcp.datasources.TestSharedPoolDataSource.getConnection(TestSharedPoolDataSource.java:49)
        [junit] at org.apache.commons.dbcp.TestConnectionPool.newConnection(TestConnectionPool.java:84)
        [junit] at org.apache.commons.dbcp.TestConnectionPool$TestThread.run(TestConnectionPool.java:595)
        [junit] at java.lang.Thread.run(Thread.java:613)
        [junit] Caused by: java.lang.NullPointerException
        [junit] at org.apache.commons.dbcp.datasources.SequencedHashMap.removeEntry(SequencedHashMap.java:217)
        [junit] at org.apache.commons.dbcp.datasources.SequencedHashMap.removeImpl(SequencedHashMap.java:471)
        [junit] at org.apache.commons.dbcp.datasources.SequencedHashMap.remove(SequencedHashMap.java:459)
        [junit] at org.apache.commons.dbcp.datasources.LRUMap.get(LRUMap.java:97)
        [junit] at org.apache.commons.dbcp.datasources.SharedPoolDataSource.getUserPassKey(SharedPoolDataSource.java:203)
        [junit] at org.apache.commons.dbcp.datasources.SharedPoolDataSource.getPooledConnectionAndInfo(SharedPoolDataSource.java:172)

        The problem is (I think) caused by concurrent modification to the LRUMap. The get method modifies the map.

        Show
        Phil Steitz added a comment - We need to protect the userkeys map. The uderlying LRUMap impl is not threadsafe. I just got this: org.apache.commons.dbcp.SQLNestedException: Could not retrieve connection info from pool [junit] at org.apache.commons.dbcp.datasources.SharedPoolDataSource.getPooledConnectionAndInfo(SharedPoolDataSource.java:183) [junit] at org.apache.commons.dbcp.datasources.InstanceKeyDataSource.getConnection(InstanceKeyDataSource.java:680) [junit] at org.apache.commons.dbcp.datasources.TestSharedPoolDataSource.getConnection(TestSharedPoolDataSource.java:49) [junit] at org.apache.commons.dbcp.TestConnectionPool.newConnection(TestConnectionPool.java:84) [junit] at org.apache.commons.dbcp.TestConnectionPool$TestThread.run(TestConnectionPool.java:595) [junit] at java.lang.Thread.run(Thread.java:613) [junit] Caused by: java.lang.NullPointerException [junit] at org.apache.commons.dbcp.datasources.SequencedHashMap.removeEntry(SequencedHashMap.java:217) [junit] at org.apache.commons.dbcp.datasources.SequencedHashMap.removeImpl(SequencedHashMap.java:471) [junit] at org.apache.commons.dbcp.datasources.SequencedHashMap.remove(SequencedHashMap.java:459) [junit] at org.apache.commons.dbcp.datasources.LRUMap.get(LRUMap.java:97) [junit] at org.apache.commons.dbcp.datasources.SharedPoolDataSource.getUserPassKey(SharedPoolDataSource.java:203) [junit] at org.apache.commons.dbcp.datasources.SharedPoolDataSource.getPooledConnectionAndInfo(SharedPoolDataSource.java:172) The problem is (I think) caused by concurrent modification to the LRUMap. The get method modifies the map.
        Mark Thomas made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Mark Thomas added a comment -

        Test case added and issue fixed. The fix will be in 1.3 onwards.

        Show
        Mark Thomas added a comment - Test case added and issue fixed. The fix will be in 1.3 onwards.
        Hide
        Mark Thomas added a comment -

        A check of DBCP confirms that getPooledConnectionAndInfo() is indeed a DBCP method so I have moved this issue to DBCP.

        Show
        Mark Thomas added a comment - A check of DBCP confirms that getPooledConnectionAndInfo() is indeed a DBCP method so I have moved this issue to DBCP.
        Mark Thomas made changes -
        Fix Version/s 2.0 [ 12311985 ]
        Project Commons Pool [ 12310488 ] Commons Dbcp [ 12310469 ]
        Key POOL-109 DBCP-291
        Issue Type Improvement [ 4 ] Bug [ 1 ]
        Fix Version/s 1.3 [ 12311977 ]
        Hide
        John Bollinger added a comment -

        I tested this against the dev code for 2.0 and 1.5, and both seem to behave as desired already. If multiple threads invoke borrowObject() while the pool is exhausted, then each one waits approximately _maxWait milliseconds before failing with a NoSuchElementException. If they all make their requests at approximately the same time, then the total time for all to finish waiting is also approximately _maxWait milliseconds (not N * _maxWait).

        It looks like this issue may have originally been filed against a different project that uses Pool. If the problem persists there then it probably belongs to that project, not to Pool.

        Show
        John Bollinger added a comment - I tested this against the dev code for 2.0 and 1.5, and both seem to behave as desired already. If multiple threads invoke borrowObject() while the pool is exhausted, then each one waits approximately _maxWait milliseconds before failing with a NoSuchElementException. If they all make their requests at approximately the same time, then the total time for all to finish waiting is also approximately _maxWait milliseconds (not N * _maxWait). It looks like this issue may have originally been filed against a different project that uses Pool. If the problem persists there then it probably belongs to that project, not to Pool.
        Hide
        Arie added a comment - - edited

        Not sure what did you have in your mind but util.concurrent (if needed) is available as concurrent.util for JDK < 1.5

        Show
        Arie added a comment - - edited Not sure what did you have in your mind but util.concurrent (if needed) is available as concurrent.util for JDK < 1.5
        Phil Steitz made changes -
        Fix Version/s 2.0 [ 12311985 ]
        Hide
        Phil Steitz added a comment -

        Improving precision in per-thread timeouts will require more advanced scheduling than is currently available. Best to wait for JDK source level 1.5.

        Show
        Phil Steitz added a comment - Improving precision in per-thread timeouts will require more advanced scheduling than is currently available. Best to wait for JDK source level 1.5.
        Phil Steitz made changes -
        Field Original Value New Value
        Project Commons Dbcp [ 12310469 ] Commons Pool [ 12310488 ]
        Key DBCP-248 POOL-109
        Arie created issue -

          People

          • Assignee:
            Unassigned
            Reporter:
            Arie
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development