Hi, I created a new branch to test the replacement of ConnectionPool, you can find it here: http://svn.geotools.org/geotools/branches/2.4-ds/
Now, I'm slowly replacing ConnectionPool with DataSource in every place it's being used, and the experience is proving worthwhile. One thing we did not consider in the initial proposal is how to close a DataSource. DataSource does not have a close method. DataSource instances coming from JNDI aren't supposed to be closed, either, but the ones we create on our own, those we have to close. Surprise surprise, the only two public methods of ConnectionPool where getConnection() and close(), and there is also a ConnectionPoolManager class that manages the created connection pools, and that caches the pools so that no two equal pools are created. Now, we're not in a position where no all DataSource can be closed, so I would propose that: * we leave the JNDI provided DataSource as is; * we create a new DataSource sub-interface, ManageableDataSource, that has a close() method * for pools we create on our own, we wrap them into a ManageableDataSource, so that it's possible to call close() on them. Each DataSource provide will have to provide its own wrapper able to really close the DataSource (for DBCP ones, BasicDataSource has a close() method available). That should be enough for most applications. ConnectionPoolManager did a little more, was a cache of ConnectionPool. Now, as it was implemented, it was quite a liability, since if you forgot to call close(ConnectionPool) it kept a reference to the created ConnectionPool objects (glab, quite a resource leak, and in the end, a good reason to run out of connections! This may really explain some troubles we have in Geoserver, since we keep soft references to DataStore objects now in order to make Geoserver scale when someone has thousands of them (if anything wrong happens, we're bound to re-create connection pools from time to time, and the result with ConnectionPoolManager is that we keep on accumulating them). Now, I'm not sure we need such a manager, in that any application should have its own way to avoid recreating over and over the same data store (being it a cache or a catalog). Yet, if you feel we still need one, then we need to: * have DataSource be registered against a DataSourceManager, that keeps a map of _weak_ references to them (so that this central manager does not become a memory leak). * the connection pool manager has its own ReferenceQueue, when a DataSource becomes weakly reachable, meaning that no hard references to it are still available, then we call close() on it and let it die. This makes sure the DataSource is closed properly. Not having a DataSourceManager would mean unclosed DataSource objects are let being garbage collected, and this should result in connections being closed anyways, thought not in the most clean way, and not in any predictable time frame (well, not that getting to the state of weakly reachable is any predictable anyways, it takes a full garbage collection to get there). Any thought? Cheers Andrea ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
