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

        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.
        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
        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
        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.
        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
        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.
        Hide
        Phil Steitz added a comment -

        Fixed threadsafety issue in r893809

        Show
        Phil Steitz added a comment - Fixed threadsafety issue in r893809

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development