Commons Dbcp
  1. Commons Dbcp
  2. DBCP-343

RemoveAbandoned setting closes the active connection.

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.3, 1.4
    • Fix Version/s: 1.3.1, 1.4.1
    • Labels:
      None
    • Environment:

      Linux 64 bit, Ubuntu 5.*

      Description

      The documentation says, RemoveAbandoned connection setting will remove only the connections that are idle for the configured timeout. But I see it is closing the connections that are active.

      The logic in DBCP implementatoin shows that it calculate the idle time from the time of the connection is created, irrespective of the connection is used. So it just close the connection in removeAbandonedTimeout.

      In my code, I'm reusing the connection for more DB statements to do intensive db operations, which hold the connection for more than 5 minutes . The connection will never be idle for more than a few seconds.

      My setting of removeabandonedTimeout = 300 brutally closes the connection in 5 minutes, without checking if the connection is in use or not.

      I assume it should involve the activity of the statement object also to do the idle time calculation.

      thanks,
      Sing

        Activity

        Hide
        Phil Steitz added a comment -

        Patch applied in trunk (r1096257) and the 1_4 branch (r1096255)

        Show
        Phil Steitz added a comment - Patch applied in trunk (r1096257) and the 1_4 branch (r1096255)
        Hide
        Phil Steitz added a comment -

        Sorry, penultimate comment above is incorrect. Using a DelegatingStatement/PreparedStatement/CallableStatement does not update the lastUsed property of the parent connection.

        removeAbandoned.patch modifies executeXxx methods of these classes update lastUsed on the parent connection. Documentation is also clarified.

        If others are OK with this patch, I will apply it to both 1.x and 2.0 branches.

        Show
        Phil Steitz added a comment - Sorry, penultimate comment above is incorrect. Using a DelegatingStatement/PreparedStatement/CallableStatement does not update the lastUsed property of the parent connection. removeAbandoned.patch modifies executeXxx methods of these classes update lastUsed on the parent connection. Documentation is also clarified. If others are OK with this patch, I will apply it to both 1.x and 2.0 branches.
        Hide
        Phil Steitz added a comment -

        If this is a bug, it should be fixed in the 1.x branch.

        Show
        Phil Steitz added a comment - If this is a bug, it should be fixed in the 1.x branch.
        Hide
        Phil Steitz added a comment -

        Patches welcome on javadoc. To be clear, it is not strictly speaking the age of the Delegating statement that counts, it when it was last used. My comment above may have been misleading. Have a look at DelegatingStatement,executeQuery() for example. If you can confirm actual JDBC operations that do not update lastUsed, then we should view this as a bug.

        Show
        Phil Steitz added a comment - Patches welcome on javadoc. To be clear, it is not strictly speaking the age of the Delegating statement that counts, it when it was last used. My comment above may have been misleading. Have a look at DelegatingStatement,executeQuery() for example. If you can confirm actual JDBC operations that do not update lastUsed, then we should view this as a bug.
        Hide
        Sebb added a comment -

        Meanwhile, ISTM that the current Javadoc could/should be updated to clarify the behaviour with long-running queries, i.e. what constitutes an "idle" connection.

        The use of the word "idle" is misleading, as currently it's the age of the DelegatingStatement object that is compared with the timeout.

        As to improving the behaviour: maybe checkOpen() should update the start time as well? This would not fix the problem entirely, but could help.

        Show
        Sebb added a comment - Meanwhile, ISTM that the current Javadoc could/should be updated to clarify the behaviour with long-running queries, i.e. what constitutes an "idle" connection. The use of the word "idle" is misleading, as currently it's the age of the DelegatingStatement object that is compared with the timeout. As to improving the behaviour: maybe checkOpen() should update the start time as well? This would not fix the problem entirely, but could help.
        Hide
        Phil Steitz added a comment -

        What you are asking for is possible. Instrumentation would have to be added to keep track of operation status of connections. Marking for 2.0.

        Show
        Phil Steitz added a comment - What you are asking for is possible. Instrumentation would have to be added to keep track of operation status of connections. Marking for 2.0.
        Hide
        Singaravelu Suburayan added a comment -

        I got it. Thanks for the details.

        Is it possible to do in this way? Is it possible to maintain the state of the connection, say executionMode or idleMode. when the execute* method of the statement is called, the connection object will be flaged with executionMode and once it is done with the operation it can come back to the idleMode. the cleanup mechanism can only closes the connections which are in idleMode

        Show
        Singaravelu Suburayan added a comment - I got it. Thanks for the details. Is it possible to do in this way? Is it possible to maintain the state of the connection, say executionMode or idleMode. when the execute* method of the statement is called, the connection object will be flaged with executionMode and once it is done with the operation it can come back to the idleMode. the cleanup mechanism can only closes the connections which are in idleMode
        Hide
        Phil Steitz added a comment -

        Each time any of the createStatement(...), prepareStatement(...), or prepareCall(...) methods are invoked, the last used property of the parent connection is updated by the DelegatingStatement constructor. Connections that execute queries that take longer than removeAbandonedTimeout to run may get closed when pool maintenance runs. If this is what you are seeing, you need to either turn off abandoned connection cleanup (always better if you can just make sure that your code actually closes all connections rather than relying on the pool to clean up after you) or set the timeout longer than the time required to execute the longest-running query that your application uses a pooled connection to execute.

        If the request here is to have DBCP periodically check the server-side active state of connections in use by clients, I am inclined to close as WONTFIX. If DBCP is not effectively updating lastUsed after JDBC operations on checked out connections, then this is a DBCP bug. In that case, details on what operations were performed and ideally a test case would be greatly appreciated.

        Show
        Phil Steitz added a comment - Each time any of the createStatement(...), prepareStatement(...), or prepareCall(...) methods are invoked, the last used property of the parent connection is updated by the DelegatingStatement constructor. Connections that execute queries that take longer than removeAbandonedTimeout to run may get closed when pool maintenance runs. If this is what you are seeing, you need to either turn off abandoned connection cleanup (always better if you can just make sure that your code actually closes all connections rather than relying on the pool to clean up after you) or set the timeout longer than the time required to execute the longest-running query that your application uses a pooled connection to execute. If the request here is to have DBCP periodically check the server-side active state of connections in use by clients, I am inclined to close as WONTFIX. If DBCP is not effectively updating lastUsed after JDBC operations on checked out connections, then this is a DBCP bug. In that case, details on what operations were performed and ideally a test case would be greatly appreciated.
        Hide
        Singaravelu Suburayan added a comment -

        With this behavior I'm not able to predict the optimal value for this setting

        Show
        Singaravelu Suburayan added a comment - With this behavior I'm not able to predict the optimal value for this setting

          People

          • Assignee:
            Unassigned
            Reporter:
            Singaravelu Suburayan
          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development