makumba / makumba

Makumba helps you rapidly develop data driven web applications. Provides a custom JSP taglib as a main interface, but leaves API open for advanced access. It is implemented in Java.
https://www.makumba.org
GNU Lesser General Public License v2.1
5 stars 2 forks source link

Encourage use of db.close() in the documentation #156

Open ghost opened 21 years ago

ghost commented 21 years ago

Reported by @frederikhabils on 18 Mar 2003 19:14 UTC In relation to bug 314 (and bug 314 comment 2 specifically), I would suggest to add this info to the API documentation

Migrated-From: http://trac.makumba.org/ticket/351

ghost commented 21 years ago

Comment by @stefanb on 21 Mar 2003 08:19 UTC OTOH it is probably confusing to suggest "Close the DB for better db handling" if a user never opened it (explicitly).

It might be more understandable to say to return the db connection (which org.makumba.Database actually is) back to the pool of available DB connections.

something like "db.release()" or so.

ghost commented 21 years ago

Comment by @cristianbogdan on 21 Mar 2003 11:12 UTC it's pretty difficult to explain to users the difference between the BL case (where a Database is passed to you and you should not close it, else the associated BL ops will be fxed up) and the findDatabase case.... that's why i didn't insist on this.

yes, we could deprecate close() and rename it to release()

ghost commented 21 years ago

Comment by @cristianbogdan on 31 Mar 2003 17:26 UTC i've head an interesting experience with a server that uses makumba connections to write events in a database. every event had a findDatabase() but no db.close () the system worked ok but it was dying now and then due to too many connections. then, after finalization and garbage collection it was getting back to life...

the point here is that db.close() seems very necessary when you use findDatabase

also, i realized that it is possible to pass (from the BL handler) to BL methods a special kind of Database objects, which are just like other DB object but are not closeable. so then we can encourage closing, and the objects provided by the BL will simply ignore or throw an exception

ghost commented 21 years ago

Comment by @frederikhabils on 18 Apr 2003 19:19 UTC it seems something similar happened just now in procuction...

I've added (in CVS but not yet in production) db.close statement on ChallengeLogic.java functions PGpointer() and CPpointer()

I really suggest to add this issue to the findDatabase() API doc.

ghost commented 21 years ago

Comment by @frederikhabils on 23 Apr 2003 07:55 UTC Just to say that the fixes mentioned in comment 4, are in P&G Challenge code, not in Johnny. And they are in production (did that day after comment 4).

ghost commented 20 years ago

Comment by @frederikhabils on 30 Jun 2003 13:32 UTC At the moment (actually, I verified this several weeks ago, not recent), all code in production that gets a Database, also does close() on it, in the manner suggested by bug 403 comment 1:

db=findDatabase(...); try { db.executeQuery() } finally( db.close(); }

It should also be noted that the technical (non-doc) part of this bug was largely solved by bug 403 which had a different cause after all.

Still, the recommendation to close() a db, is still valid as far as i know.

ghost commented 20 years ago

Comment by @cristianbogdan on 23 Feb 2004 20:40 UTC actually i think the best recommendation would be

db=MakumbaSystem.getConnectionTo(...); try { .... } catch(Throwable t){ db.rollback(); } finally( db.close(); }

i am just working with an Access db which seems not to be so reliable, probably throws exceptions and leaves the db in all sorts of inconsistent states. the problem is that if an exception occurs in the try{} block, the db.close() will commit, and without the catch/rollback, the chances to commit an inconsistent state are pretty high...

the above catch/rollback is done automatically by business logic (look in org.makumba.controller.Logic), but my app is not a webapp hence it has no BL... also, all explicit calls to getConnectionTo() (or the old findDatabase()) suffer from the same problem. OK, when just a query is executed (as in comment 6) that's no problem, but if inserts/updates/deletes are executed inside the try{} the problem is serious.

i guess i forgot about the catch () because i knew that makumba does rollback automatically, but i forgot it's in BL which i don't have.

ghost commented 20 years ago

Comment by @cristianbogdan on 23 Feb 2004 20:49 UTC Actually things are a bit more complex because you need to throw the errors and exceptions further for things to work as before. Since the original try{} has no catch, we can assume that the only things thrown are Throwables that need not be caught, i.e. Errors (incl MakumbaError) and RuntimeExceptions. So:

db=MakumbaSystem.getConnectionTo(...); try { .... } catch(Error e){ db.rollback(); throw e; } catch(RuntimeException re){ db.rollback(); throw re; } finally{ db.close(); }

ghost commented 13 years ago

Modified by @manuelbernhardt on 27 Jun 2010 19:43 UTC

ghost commented 13 years ago

Modified by @manuelbernhardt on 28 Jun 2010 07:24 UTC

ghost commented 13 years ago

Modified by @manuelbernhardt on 28 Jun 2010 07:36 UTC