Hello,
I have been working on the connection pooling mechanism of Turbine lately
and I have
discovered some bugs. I am not sure how I can give feedback or corrections
so I am
writing to this e-mail these problems.

My primary goal was to be able to make the ConnectionPool mechanism recover
after
a full database shutdown and restart. It is obvious that on a production
system (24X7X365)
it is very important for the applications to be able to recover from a db
restart.

The first correction I made was the following:

    protected DBConnection getNewConnection()
        throws Exception
    {
            //old code
        //totalConnections++;
        //return new DBConnection( getDB().getConnection(), url );

            //new code
        DBConnection dbCon = new DBConnection( getDB().getConnection(),
url );
        totalConnections++;
        return dbCon;
    }

It is obvious here that if the db is closed (or the DB adapter cannot create
for some
reason a connection, e.g. network failure) the totalConnections counter
should not
be incremented (since the line new DBConnection( getDB().getConnection(),
url ) will
throw an exception).

The following is a test case. I use an oracle db and I shutdown and restart
the db. The
above correction makes the Connection recover.

   public static void TurbineTest1(){
         try
         {
            TurbineConfig config = new TurbineConfig("c:/util/javatools",
"TR-14-02.properties");
            config.init();
            DBConnection dbcon = null;
            Connection con = null;
            for(int i=0; i<100000; i++){
                try{
                    dbcon = TurbineDB.getConnection();
                    con = dbcon.getConnection();
                    Statement s = con.createStatement();
                    s.close();
                    System.out.println("Connection: " + con);
                    Thread.currentThread().sleep(200);
                }catch(Exception e){
                    try{
                        con.close();
                    }catch(Exception pe){};
                    System.out.println("Connection to db failed: " + e);
                }
                finally{
                    try{
                        TurbineDB.releaseConnection(dbcon);
                    }catch(Exception e){
                        System.err.println("Error releasing connection: " +
e);
                    };
                }
            }

            System.in.read();
         }
         catch (Exception e)
         {
            System.err.println("Error: " + e);
         }
     }

There is still one problem though. The block
                    try{
                        con.close();
                    }catch(Exception pe){};
                    System.out.println("Connection to db failed: " + e);
is still necessary in order to mark somehow the connection as invalid;
actually
there is no other way to mark a connection as invalid than closing it
explicitly.
This has a drawback: the TurbineDB.releaseConnection(dbcon) method will
throw an exception since the actual connection is closed. This means, that
the connection will never return to the pool. To be exact, the above code
will
recover n db restarts where n is the size of the pool (each time an error
occurs
we will have a hanging connection that counts as active but  can
never be returned to the pool). After that, the getConnection() method will
always throw a ConnectionTimeoutException...

Generally, the above code should always be used to invalidate, somehow,
connections
that have irrecoverable errors (for example, if the Statement in the above
code is
not closed - s.close(); - oracle will throw a "Too many open cursors" error
after
m attempts (m: configured on oracle db) and this connection will not ever be
able
to recover...

There are a couple of ways to work-around this issue: the point is that
somehow
there should be a way to mark a connection as invalid and still return it to
the pool
through the TurbineDB.releaseConnection(DBConnection) method: the
ConnectionPool
should then discard completely of this connection (instead of returning it
to the pool).

A simple work-around is to have the releaseConnection method of
ConnectioPool not
throw an Exception when the physical connection is closed. The Exception
occurs in the
isValid method of the ConnectionPool class (and more specifically in the
DBConnection.getConnection() method). Actually, if you look closely to these
two methods
you will see a paradox: the DBConnection.getConnection() method will never
return null
or a connection that is closed... (it throws exception) but the
ConnectionPool.isValid()
method checks for these return values. The same paradox applies to the
DBConnection.close()
method that attempts to get the connection and then checks if it is null or
closed...

I am not sure whether the DBConnection.getConnection() method should be
changed
to not check if the connection is null or closed() or change the
ConnectionPool.isValid()
and the DBConnection.close() method to catch these Exception and close
anyway..

On the other hand, the releaseConnection method of ConnectionPool class
tries to close
the connection if it is invalid (meaning that being valid and closed are two
separate things).
So maybe a new method DBConnection.invalidate() should be added to mark a
connection
as invalid.

I am not the expert to say, so please, give me some feedback to correct the
above issue.

Two more (minor) issues:
The TurbineDB.getDB(String name) method should be changed to call the
getPoolBroker.getDB(String)
method:
    public static DB getDB(String name)
        throws Exception
    {    //old code
        //return getPoolBroker().getDB();

        //new code
        return getPoolBroker().getDB(name);
    }


The ConnectionPool.getPooledConnection() method(s) should be synchronized.
(I think so)

Here are some modified version of isValid() and releaseConnection() of the
ConnectionPool class
that will make the above code work (if without the s.close() statement):

    private boolean isValid( DBConnection connection )
        throws Exception
    {
        // Test whether the connection is valid and return the result.
//        return ( connection.getConnection() != null &&
//                 !connection.getConnection().isClosed() &&
//                 !isExpired(connection) );

            try
            {
                // just attempt to get the connection: if it is invalid, the
                // getConnection() method will throw SQLException
                connection.getConnection();
                return !isExpired(connection);
            }
            catch(SQLException sqle)
            {
                return false;
            }
    }



    public synchronized void releaseConnection(DBConnection dbconn)
        throws Exception
    {
        // DBConnections MUST be unlinked when returned to the pool
        dbconn.unlink(this);

        if ( isValid(dbconn) )
        {
            pool.push(dbconn);
            notify();
        }
        else
        {
            //ignore error at this stage: you can rethrow it after
decrementConnection() if you want to...
            try{
                dbconn.close();
            }catch(SQLException ignore){};

            decrementConnections();
        }
    }



Sorry for the big e-mail, but I have no other way to communicate problems
back to the list.
Thanks for you patience

Costas





------------------------------------------------------------
To subscribe:        [EMAIL PROTECTED]
To unsubscribe:      [EMAIL PROTECTED]
Search: <http://www.mail-archive.com/turbine%40list.working-dogs.com/>
Problems?:           [EMAIL PROTECTED]

Reply via email to