Code review comment for lp:~florent.x/openobject-server/trunk-bug-905257-fix-reconnect

Revision history for this message
Florent (florent.x) wrote :

Hi Olivier,

Thank you for your feedback on this patch.
I reviewed your changes, technically it fixes the same issue as this MP.

However, I am not certain that it is a better approach.
Let's imagine that you have a pool of 16 connections. Each time you request a cursor(), the server will check the 16 connections and call cnx.reset() on each one.
According to the psycopg documentation, the reset() executes 2 commands: RESET and SET SESSION AUTHORIZATION. It's not really a no-op.
So you will execute 32 commands each time you grab a cursor, just to be sure that all connection is available. It is a bit suboptimal.

Finally, I still prefer my original patch: "easier to ask for forgiveness than permission"

BTW, there's a mismatch between the comment and the code here (and the comment is right):
http://bazaar.launchpad.net/~openerp/openobject-server/7.0/revision/4837.1.2/openerp/sql_db.py

« Back to merge proposal