Bug report #7679

DBManager does not release PostGIS connections

Added by Nyall Dawson almost 11 years ago. Updated over 9 years ago.

Status:Closed
Priority:Normal
Assignee:-
Category:DB Manager
Affected QGIS version:master Regression?:No
Operating System: Easy fix?:No
Pull Request or Patch supplied:Yes Resolution:fixed/implemented
Crashes QGIS or corrupts data:No Copied to github as #:16596

Description

In current master the connection to a PostGIS database is never released, even after all PostGIS layers have been removed from a map. The only way to close the connection and release the PostGIS layer for editing in other clients is to completely close QGIS.

I'm not at all familiar with the postgres provider code, but it appears to me to be related to the check

  if ( --mRef > 0 )
    return;

on line 260 of qgspostgreconn.cpp.

In my testing, mRef never reaches zero. Adding a single postgis layer to a map results in an mRef value of 2. Removing the postgis layer drops this to 1, but it never hits 0 and as a result the connection is never removed and deleted.

locked_view.png (35.7 KB) Nyall Dawson, 2013-10-27 02:47 PM

connection-holding-fix.patch Magnifier - patch to fix connection holding issue (1.65 KB) Ivan Mincik, 2014-05-02 05:37 AM

Associated revisions

Revision 5f5cd4cc
Added by Giuseppe Sucameli over 10 years ago

dbmanager: be sure to close db cursors (fix #7679)

History

#1 Updated by Jürgen Fischer almost 11 years ago

Are you using the browser?

#2 Updated by Nyall Dawson almost 11 years ago

Yes, I was using the browser - that explains why mref never reached zero.

However, there's something else at play here. I've removed the browser, and now the connections.remove method is called. However, QGIS still seems to hang on to a connection to the view/database.

To explain in more detail:

1. I start a new project and add a PostGIS view "census.vw_birthplace" to my map
2. I remove this layer. connections.remove is called by qgspostgreconn.cpp
3. A process remains active on the Postgresql database from QGIS - in this case the process is "SELECT pg_get_viewdef(c.oid) FROM pg_class c JOIN pg_namespace nsp ON c.relnamespace = nsp.oid WHERE relname='vw_birthplace' and nspname='census' and relkind='v'"
4. This process blocks any further changes to the census.vw_birthplace view
5. Closing QGIS removes the blocking process and allows changes to census.vw_birthplace view

#3 Updated by Nyall Dawson almost 11 years ago

Similarly, if I repeat the process with a table instead of a view the blocking process is:

"SELECT rulename, definition FROM pg_rules WHERE tablename='census_simplified' and schemaname='census'"

#4 Updated by Jürgen Fischer almost 11 years ago

  • Category changed from Data Provider/PostGIS to DB Manager

Both queries are apparently is from the db_manager plugin.

#5 Updated by Nyall Dawson almost 11 years ago

Ok, I can confirm that -- if I don't use the DB_manager plugin to add the postgis layers then the connection is correctly released when the layer is removed.

#6 Updated by Nyall Dawson over 10 years ago

  • Subject changed from PostGIS connection is never released to DBManager does not release PostGIS connections

#7 Updated by Alexandre Neto over 10 years ago

I can confirm this issue in QGIS 2.0 64bit in Windows 7.

#8 Updated by Giuseppe Sucameli over 10 years ago

  • Status changed from Open to Closed

#9 Updated by Nyall Dawson over 10 years ago

Unfortunately I'm still seeing this after 5f5cd4c. I've attached a screenshot which shows the issue in PG Admin. The steps to reproduce are:

1. Use DB Manager to select a view (in this case "public.vw_test_7679")
2. Drag and drop that view to the map. DB manager runs a query "SELECT pg_get_viewdef(c.oid) FROM pg_class ...." (process 7296 in the attached screenshot)
3. Close DB Manager, remove the view layer from the map -- the connection for process 7296 is still open
4. Attempt to edit the view in PG Admin but it is locked by process 7296.
5. Closing QGIS closes the connection and releases the lock

I'm happy to try any diagnose/debugging steps you'd like to help track this down.

#10 Updated by Ivan Mincik almost 10 years ago

Nyall, DB Manager connects with default isolation level which leaves connection in (IDLE in transaction). You can try to change it in PostGisDBConnector class by setting

self.connection.set_isolation_level(psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT)

Now you can edit tables in PG Admin when QGIS and even DB Manager are running, but some other things which require for example "named cursors" will not work.

#11 Updated by Ivan Mincik almost 10 years ago

By my quick test only displaying data from table is not working when setting ISOLATION_LEVEL_AUTOCOMMIT for connection.

#12 Updated by Ivan Mincik almost 10 years ago

Here is a working patch which will fix connection holding issue by setting DB Manager connection to AUTOCOMMIT isolation level (which is better practice). I have removed named cursor usage in PGTableDataModel._createCursor to fix problems when running with AUTOCOMMIT.

After few minutes of testing all functions it seems OK. Please test.

#13 Updated by Ivan Mincik almost 10 years ago

  • Assignee set to Giuseppe Sucameli

#14 Updated by Giovanni Manghi almost 10 years ago

  • Status changed from Reopened to Feedback
  • Pull Request or Patch supplied changed from No to Yes

Ivan Mincik wrote:

Here is a working patch which will fix connection holding issue by setting DB Manager connection to AUTOCOMMIT isolation level (which is better practice). I have removed named cursor usage in PGTableDataModel._createCursor to fix problems when running with AUTOCOMMIT.

After few minutes of testing all functions it seems OK. Please test.

Hi!

it would be better to make a Pull Request on github with your patch, otherwise the risk is that it will not be reviewed.

Thanks!

#15 Updated by Paolo Cavallini almost 10 years ago

  • Assignee deleted (Giuseppe Sucameli)

#17 Updated by Giuseppe Sucameli over 9 years ago

  • Status changed from Feedback to Closed
  • Resolution set to fixed/implemented

Your pull request was merged in d522059524. Thanks Ivan.

Also available in: Atom PDF