Github user jburwell commented on the pull request:

    https://github.com/apache/cloudstack/pull/689#issuecomment-134245543
  
    @abhi An connection pool makes acquiring a scarce resource less expensive, 
but it is not cost-free.  By taking a connection when the pool is under load, 
the following could occur:
    
    * This code may request a connection when none are available causing the 
connection pool to create another ad hoc connection or blocking the thread 
until a connection becomes available -- dependent on the pool configuration.
    * This code may exhaust the last connection in the pool causing other 
thread(s) to create ad hoc connection(s) or to block waiting for a connection 
to become available -- dependent on the pool configuration.
    
    There are variety of patterns that can be employed to address this problem. 
  I lean towards a construct such as the following:
    
    ```
    Transaction transaction = new Transaction(USAGE_DB).attempt(new 
TransactionWork() {
    
          public void execute(final Connection connection) throws SQLException{
    
                 // Do database work ...
          }
    
    }
    ```
    
    The ``Transaction.attempt`` method wraps the call to the passed 
``TransactionWork.execute`` method with the following boilerplate that appears 
throughout the code base:
    
    1. Capturing the current database ID from ``TransactionLegacy``
    2. Open a connection to the requested database
    3. Handling exceptions including translation and proper logging
    4. Aborting/commiting/closing the transaction via ``TransactionLegacy`` as 
needed
    5. If necessary, switching ``TransactionLegacy`` back to the previous 
database ID
    
    These steps assume that ``TransactionLegacy`` has a ``switch`` method that 
changes the ``ThreadLocal`` tracking the current database ID without consuming 
a connection from the pool.
    
    I prefer this approach because it allows DAO to declare transactional 
blocks in a side-effect free manner.  This form of code should be immune to a 
replacement of ``TransationLegacy`` because it only relies on the JDBC API and 
makes no assumptions about the call flow.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to