BTM
  1. BTM
  2. BTM-66

Memory retention problem due JdbcPooledConnection uncachedStatements list

    Details

    • Type: Bug Bug
    • Status: Closed Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 1.3.3
    • Fix Version/s: 2.0.0
    • Labels:
      None
    • Environment:
      Hibernate3.5, SQLServer
    • Number of attachments :
      1

      Description

      I face a memory retention problem with JdbcPooledConnection uncachedStatements list.
      Problem is that this list keeps hard references to Statements even after they had been closed by the user.
      The list is only cleared when releasing the connection back to the pool.
      On long transaction with many queries this may lead to a serious memory consumption problems.

      My scenario:
      During a single transaction I perform many queries.
      After 9653 queries I create a dump and analyzing it with SAPMemoryAnalyzer and JHAT I saw following:
      -115MB of total 133MB are used by the 9653 SQLServerPreparedStatements
      -making some spot test I see that all SQLServerPreparedStatements are already closed
      -the unique referrer to this SQLServerPreparedStatements is the JdbcPooledConnection uncachedStatements list

      Workaround: setting PreparedStatementCacheSize to a value > 0 resolved the problem to me,
      (but I assume that it doesn't work for all scenarios)

      ---------------------------------------------------------------------------------------------
      First answer by Ludovic:
      Keeping a list of all opened statements is required for compliance with JDBC spec which says if a connection gets closed all associated resources should be closed as well. Another workaround would be to close the connection and open another one from time to time in your transaction.

        Activity

        Hide
        Brett Wooldridge added a comment -

        Ludovic and Guenther,

        After thinking about this a bit, I don't think we should track uncached statements. Our connection should be almost completely transparent to the user. The underlying driver connection (be it Derby, MySQL, Oracle, etc.) is required by the spec. to track statements and close associated resources when the connection closes – it is not our responsibility to do so. If the user obtains a statement (uncached) from our connection, we are essentially a pass-thru to the underlying connection. If the user closes that statement and releases their reference to it, BTM shouldn't care one way or another.

        If the user elects to use connection pooling, their expectation should be that the connection itself may not be closed for an indeterminate period of time, and therefore they are responsible for the closing of statements.

        However, if we do wish to track uncached statements, those statements should be wrapped in a proxy such that when they are closed explicitly by the user they are removed from our tracking list. Doing so allows resources, including ResultSets, to be freed and garbage collected in a timely manner. In this case, our tracking list wouldn't be a list of uncached statements, but rather a list of "un-closed uncached" statements.

        "Un-closed uncached" statements would be closed explicitly when our connection closes or is returned to the pool.

        Show
        Brett Wooldridge added a comment - Ludovic and Guenther, After thinking about this a bit, I don't think we should track uncached statements. Our connection should be almost completely transparent to the user. The underlying driver connection (be it Derby, MySQL, Oracle, etc.) is required by the spec. to track statements and close associated resources when the connection closes – it is not our responsibility to do so. If the user obtains a statement (uncached) from our connection, we are essentially a pass-thru to the underlying connection. If the user closes that statement and releases their reference to it, BTM shouldn't care one way or another. If the user elects to use connection pooling, their expectation should be that the connection itself may not be closed for an indeterminate period of time, and therefore they are responsible for the closing of statements. However, if we do wish to track uncached statements, those statements should be wrapped in a proxy such that when they are closed explicitly by the user they are removed from our tracking list. Doing so allows resources, including ResultSets, to be freed and garbage collected in a timely manner. In this case, our tracking list wouldn't be a list of uncached statements, but rather a list of "un-closed uncached" statements. "Un-closed uncached" statements would be closed explicitly when our connection closes or is returned to the pool.
        Hide
        Brett Wooldridge added a comment -

        Guenther,

        While I agree this is an issue, and BTM should fix it (either by cleaning up our resource tracking list when statements are closed, or by not tracking statements), I do strongly recommend using the PreparedStatement cache. It will not only reduce your memory consumption, but should increase performance considerably. By using it you will avoid several thousand roundtrips to the database (in your scenario), and will allow the database to re-use the same execution plan for each identical statement.

        Show
        Brett Wooldridge added a comment - Guenther, While I agree this is an issue, and BTM should fix it (either by cleaning up our resource tracking list when statements are closed, or by not tracking statements), I do strongly recommend using the PreparedStatement cache. It will not only reduce your memory consumption, but should increase performance considerably. By using it you will avoid several thousand roundtrips to the database (in your scenario), and will allow the database to re-use the same execution plan for each identical statement.
        Hide
        Brett Wooldridge added a comment -

        Attached patch file to fix BTM-66.

        Using the recently added BaseProxyHandlerClass this patch adds proxies for uncached Statement, CallableStatement, and PreparedStatement classes that override the close() method of each of those classes respectively. When a close explicitly occurs by the user calling close(), the statement is removed from the tracking list of unclosed uncached resources and the close() method on the underlying statement is called.

        This allows BTM to quickly release a resource in the event that the user proactively closes the resource, but still allows BTM to close dangling resources when the underlying connection is closed (or returned to the pool).

        All BTM unit tests are passing.

        Show
        Brett Wooldridge added a comment - Attached patch file to fix BTM-66 . Using the recently added BaseProxyHandlerClass this patch adds proxies for uncached Statement, CallableStatement, and PreparedStatement classes that override the close() method of each of those classes respectively. When a close explicitly occurs by the user calling close(), the statement is removed from the tracking list of unclosed uncached resources and the close() method on the underlying statement is called. This allows BTM to quickly release a resource in the event that the user proactively closes the resource, but still allows BTM to close dangling resources when the underlying connection is closed (or returned to the pool). All BTM unit tests are passing.
        Hide
        Brett Wooldridge added a comment -

        Guenther,

        I built a test version of Bitronix from the head which includes this patch if you want to test it and report back the results.

        You can download it at: http://www.dancernetworks/btm-1.3.4.zip

        Note that because it was built from the head, it includes other unreleased fixes and features, so be aware of that if you choose to use it in production.

        Show
        Brett Wooldridge added a comment - Guenther, I built a test version of Bitronix from the head which includes this patch if you want to test it and report back the results. You can download it at: http://www.dancernetworks/btm-1.3.4.zip Note that because it was built from the head, it includes other unreleased fixes and features, so be aware of that if you choose to use it in production.
        Hide
        Guenther Demetz added a comment -

        Thank you Brett, I tested it right now and the memory retention problem arises no more.

        Show
        Guenther Demetz added a comment - Thank you Brett, I tested it right now and the memory retention problem arises no more.
        Hide
        Ludovic Orban added a comment -

        Then it's a good inclusion for the next release. I'll review and commit the patch myself.

        Thanks!

        Show
        Ludovic Orban added a comment - Then it's a good inclusion for the next release. I'll review and commit the patch myself. Thanks!
        Hide
        Ludovic Orban added a comment -

        patch applied.

        Show
        Ludovic Orban added a comment - patch applied.

          People

          • Assignee:
            Ludovic Orban
            Reporter:
            Guenther Demetz
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: