Could you create a diff with CVS against your changes then post that diff
so we can use it?
cvs diff -u
Everyone is used to getting diff's and then it is easier to just apply that
to the CVS tree also.
jb
> -----Original Message-----
> From: ������ �������� [mailto:[EMAIL PROTECTED]]
> Sent: Thursday, February 15, 2001 4:29 AM
> To: Turbine
> Subject: ConnectionPool Bugs
>
>
> 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]
>
-----------------------------------------------------------------------
This message has been scanned for viruses with Trend Micro's Interscan VirusWall.
------------------------------------------------------------
To subscribe: [EMAIL PROTECTED]
To unsubscribe: [EMAIL PROTECTED]
Search: <http://www.mail-archive.com/turbine%40list.working-dogs.com/>
Problems?: [EMAIL PROTECTED]
