Re: [Openerp-community-reviewer] [Merge] lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn into lp:ocb-server
Review: Disapprove Development for 7.0 has moved to github on https://github.com/OCA/ocb - please move your merge proposal there if it is still valid. (I close and reject this in order to have a cleaner overview for 6.1 MPs which indeed have to be done on launchpad) -- https://code.launchpad.net/~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn/+merge/220666 Your team OpenERP Community Backports is subscribed to branch lp:ocb-server. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
[Openerp-community-reviewer] [Merge] lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn into lp:ocb-server
The proposal to merge lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn into lp:ocb-server has been updated. Status: Needs review = Rejected For more details, see: https://code.launchpad.net/~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn/+merge/220666 -- Your team OpenERP Community Backports is subscribed to branch lp:ocb-server. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn into lp:ocb-server
Review: Approve code review, sometest without being able to exercise that code LGTM : the connection must be closed. But I cannot find by reading the code a case where this branch is executed (i.e. a logical path in which we get len(self._connections) = self._maxconn so I'm wondering if we are not improving dead code here. Do you have a scenario at hand? -- https://code.launchpad.net/~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn/+merge/220666 Your team OpenERP Community Backports is subscribed to branch lp:ocb-server. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn into lp:ocb-server
Thanks for review. What I understand by reading the code is that this function has 4 parts. First it removes closed or leaked connections - none of our business here Then it tries to re-use an iddle connection previously opened for the same database If no iddle connection is found for this database, and if db_maxconn is reached, it will free any other iddle connection, and here was the bug Finally, it will open a new connection So the bug needs to have several databases opened to occur. How to test : I check number of postgres connections on linux with the psql command: psql -c SELECT datid,datname,procpid,usesysid,usename,current_query FROM pg_stat_activity WHERE application_name!='psql'; postgres Set in config file db_maxconn = 4 (must be higher than max_cron_threads if I remember correct) Run openerp Login to a database You should see 3 or 4 postgres connections for your database. Now, without loging out, login to another database (loging out would free connections) You should now see more than 4 used databases. If not, click on a menu on the 2 databases with a low time interval. For whatever reason connections are correctly freed or their database switched sometimes, but I don't know why or where in the code. -- https://code.launchpad.net/~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn/+merge/220666 Your team OpenERP Community Backports is subscribed to branch lp:ocb-server. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn into lp:ocb-server
thanks for the precision. -- https://code.launchpad.net/~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn/+merge/220666 Your team OpenERP Community Backports is subscribed to branch lp:ocb-server. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
Re: [Openerp-community-reviewer] [Merge] lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn into lp:ocb-server
Review: Approve code review LGTM. Thanks for the patch. Regards. -- https://code.launchpad.net/~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn/+merge/220666 Your team OpenERP Community Backports is subscribed to branch lp:ocb-server. -- Mailing list: https://launchpad.net/~openerp-community-reviewer Post to : openerp-community-reviewer@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community-reviewer More help : https://help.launchpad.net/ListHelp
[Openerp-community-reviewer] [Merge] lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn into lp:ocb-server
Cedric Le Brouster(OpenFire) has proposed merging
lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn into
lp:ocb-server.
Requested reviews:
OpenERP Community Backports (ocb)
Related bugs:
Bug #1322191 in OpenERP Server: db_maxconn config parameter doesn't work
https://bugs.launchpad.net/openobject-server/+bug/1322191
For more details, see:
https://code.launchpad.net/~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn/+merge/220666
Close database connections before removing them.
Fixes bug lp:1322191
--
https://code.launchpad.net/~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn/+merge/220666
Your team OpenERP Community Backports is requested to review the proposed merge
of lp:~cedric-lebrouster/ocb-server/ocb-7.0-bug-1322191-db_maxconn into
lp:ocb-server.
=== modified file 'openerp/sql_db.py'
--- openerp/sql_db.py 2014-04-09 15:16:59 +
+++ openerp/sql_db.py 2014-05-22 14:50:38 +
@@ -430,6 +430,8 @@
for i, (cnx, used) in enumerate(self._connections):
if not used:
self._connections.pop(i)
+if not cnx.closed:
+cnx.close()
self._debug('Removing old connection at index %d: %r', i, cnx.dsn)
break
else:
--
Mailing list: https://launchpad.net/~openerp-community-reviewer
Post to : openerp-community-reviewer@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openerp-community-reviewer
More help : https://help.launchpad.net/ListHelp
