OGR provider: only enable journal_mode = wal when editing. requires GDAL >= 3.4.2 (fixes #23991) #47098

rouault · 2022-01-31T16:56:00Z

This requires the NOLOCK open option of the GPKG driver added per OSGeo/gdal#5207
For earlier GDAL version, previous behaviour is kept.

With GDAL >= 3.4.2, when creating a QgsOgrProvider object, we first open
it in update mode without forcing WAL, to get the appropriate
capabilities, close it, and re-open it in read-only mode with the
NOLOCK=YES open option. This option will only be honoured if the file
wasn't already in WAL mode.
When editing a layer, the file is re-opened in update mode and with
enabling WAL to avoid blocking between readers and writers.
When closing a file, journal mode is attempted to be reset to DELETE as
before.

I've verified that test_provider_ogr_gpkg.py and test_provider_ogr.py
pass locally with GDAL master + OSGeo/gdal#5207

Could be worth backporting to LTR, but perhaps would require some additional testing from others

nirvn · 2022-01-31T16:59:33Z

I'd love to see this backported. Maybe after it lives on 3.24 for a month or so.

gioman · 2022-01-31T17:28:22Z

I'd love to see this backported. Maybe after it lives on 3.24 for a month or so.

Agree, especially if this means that the GPKG open in read only mode will stop to be marked as changed on the file system.

…DAL >= 3.4.2 (fixes qgis#23991) This requires the NOLOCK open option of the GPKG driver added per OSGeo/gdal#5207 For earlier GDAL version, previous behaviour is kept. With GDAL >= 3.4.2, when creating a QgsOgrProvider object, we first open it in update mode without forcing WAL, to get the appropriate capabilities, close it, and re-open it in read-only mode with the NOLOCK=YES open option. This option will only be honoured if the file wasn't already in WAL mode. When editing a layer, the file is re-opened in update mode and with enabling WAL to avoid blocking between readers and writers. When closing a file, journal mode is attempted to be reset to DELETE as before. I've verified that test_provider_ogr_gpkg.py and test_provider_ogr.py pass locally with GDAL master + OSGeo/gdal#5207

src/core/providers/ogr/qgsogrprovider.cpp

kannes · 2022-02-01T12:40:41Z

This would fix #23991, thank you so much in advance.

florisvdh · 2022-02-02T08:11:26Z

Thank you SO very much for handling this! 👍 🎉 Looking forward to the next releases.

FYI @paleolimbot

gdt · 2022-02-02T13:32:29Z

When a file is opened as NOLOCK=yes, does that really mean no locking? Isn't it then the case that reading from the geopackage is unsound if some other program has it open?

I think an underlying issue is likely a difference of opinion as to the nature of geopackage files. qgis seems to view them as private data files within the qgis world, as opposed to databases that can be accessed by others, or at least that's how it seems reading the discussions. sqlite is an odd case in databases, as it can be both private storage for a program (perhaps the more common case) as well as functioning like a regular database.

rouault · 2022-02-02T14:08:59Z

When a file is opened as NOLOCK=yes, does that really mean no locking?

yes

Isn't it then the case that reading from the geopackage is unsound if some other program has it open?

see bottom of comment of OSGeo/gdal#5207 (comment)

agiudiceandrea · 2022-03-24T16:24:14Z

@rouault tested with QGIS 3.24.0 and GDAL 3.4.2 on Windows:

it seems not possible to set NOLOCK=YES as default for GPKG files dragged and dropped in QGIS
the possibility to set the NOLOCK=YES for GPKG files is only available if the GPKG file is added using the Add Vector Layer window
Is this behavior expected? If yes, could it be improved in order to let the user set the NOLOCK=YES option as default for dragged and dropped GPKG files?

rouault · 2022-03-24T16:30:59Z

it seems not possible to set NOLOCK=YES as default for GPKG files dragged and dropped in QGIS. the possibility to set the NOLOCK=YES for GPKG files is only available if the GPKG file is added using the Add Vector Layer window. Is this behavior expected? If yes, could it be improved in order to let the user set the NOLOCK=YES option as default for dragged and dropped GPKG files?

The NOLOCK=YES open option is mostly an internal detail. Users shouldn't need to set it generally. The provider is supposed to do the right thing, and set it automatically when the layer is in read-only mode, whatever how it was added to the project

agiudiceandrea · 2022-03-24T16:36:06Z

The provider is supposed to do the right thing, and set it automatically when the layer is in read-only mode, whatever how it was added to the project

It seems to me it is not the case. Adding a GPKG layer (not opened by any other process) with drag and drop or using the Add Vector Layer window and leaving the NOLOCK option as <Default>, then the whal and shm files are created and the modify date of the gpkg file is changed even if the layer in QGIS is not edited.
Did I miss something or I misunderstand the point of this PR?

rouault · 2022-03-24T17:03:02Z

Has your QGIS version being rebuilt against GDAL 3.4.2 headers (and not just using it at runtime) ?

agiudiceandrea · 2022-03-24T17:19:39Z

Has your QGIS version being rebuilt against GDAL 3.4.2 headers (and not just using it at runtime) ?

Actually not. I'm using the GDAL 3.4.2 runtime with QGIS 3.24.0 built against GDAL 3.4.1. So it should is this the issue. Then I will try again with a QGIS version built against GDAL 3.4.2. Thanks for the clarification. I apologise for the noise...

agiudiceandrea · 2022-03-28T07:04:10Z

Hi ~~rduivenvoorde~~ @rouault, I've tested this patch with QGIS 3.24.1 (qgis-3.24.1-2 from OSGeo4W on Windows 10) built against GDAL 3.4.2 and using GDAL 3.4.2 runtime DLL.

The behaviour seems correct to me when a GPKG file is dragged and dropped in QGIS or added to a map using the Browser panel (in such cases the whal and shm files are not created and the modify date of the gpkg file is not changed).

If the GPKG file is added to a map using the Vector Layer window, then the behaviour is always the same (whal and shm files are not created and the modify date of the gpkg file is not changed) however the user sets the NOLOCK option (Yes, No, <Default>).
Is this behaviour expected? If yes, what's the point to expose the NOLOCK option to the user in QGIS?

rduivenvoorde · 2022-03-28T08:45:45Z

@agiudiceandrea I was mentioned in your comment, but you probably meant @rouault ?
Or do you want me to double check something...?

agiudiceandrea · 2022-03-28T09:04:57Z

I was mentioned in your comment

Please, excuse me! I meant to tag Even Rouault.

This option is automatically set by the OGR provider. No need for the user to mess with it. Refs qgis#47098 (comment)

rouault · 2022-03-28T10:20:50Z

If yes, what's the point to expose the NOLOCK option to the user in QGIS?

it shouldn't be exposed (it is currently, as it is done automatically since QGIS auto-discovers open options from GDAL driver metadata). Fixed per #47992

This option is automatically set by the OGR provider. No need for the user to mess with it. Refs #47098 (comment)

agiudiceandrea · 2022-03-31T14:57:00Z

Hi @rouault, it seems to me the issues reported at #48003 and #48024 could be related to this PR.

This option is automatically set by the OGR provider. No need for the user to mess with it. Refs qgis/QGIS#47098 (comment)

Fixes qgis#48012 Basically, for offline editing, we ask QgsOgrProvider to adopt the behaviour it had before qgis#47098, that is use SQLite3 journal_mode = WAL even in read-only mode. Sorry, this "fix" is horrible, but I couldn't get with something better after many hours of investigation. The root cause seems to be that QgsOfflineEditing creates layers using direct OGR API or sqlite3 API, and also uses QgsOgrProviderUtils which has a GDAL dataset reuse mechanism. And all that does not place nicely at all. Maybe the root root cause is that the OGR GPKG driver establishes the list of layers at its opening, and if creates a new one behind its back, which is prone to happen with the dataset caching metchanism, the GDALDataset::GetLayerByName() method doesn't see the newly created layer. With that in mind, I tried to add calls to QgsOgrProviderUtils::invalidateCachedDatasets() but to no avail. Also QgsOgrProviderUtils::invalidateCachedLastModifiedDate() which is used by QgsOgrProvider::createEmptyLayer(), but to no avail. So basically I don't understand why offline editing worked before with a GPKG backend. I'd say it is pure luck. Note: I also saw that qgsofflineediting.cpp at line 315 calls offlineLayer->dataProvider()->invalidateConnections() but that method is only implemented in the Spatialite provider. I tried to implement it in the OGR one, but to no avail too.

Fixes qgis#48012 Basically, for offline editing, we ask QgsOgrProvider to adopt the behaviour it had before qgis#47098, that is use SQLite3 journal_mode = WAL even in read-only mode. Sorry, this "fix" is horrible, but I couldn't get with something better after many hours of investigation. The root cause seems to be that QgsOfflineEditing creates layers using direct OGR API or sqlite3 API, and also uses QgsOgrProvider which has a GDAL dataset reuse mechanism. And all that does not place nicely at all. Maybe the root root cause is that the OGR GPKG driver establishes the list of layers at its opening, and if creates a new one behind its back, which is prone to happen with the dataset caching metchanism, the GDALDataset::GetLayerByName() method doesn't see the newly created layer. With that in mind, I tried to add calls to QgsOgrProviderUtils::invalidateCachedDatasets() but to no avail. Also QgsOgrProviderUtils::invalidateCachedLastModifiedDate() which is used by QgsOgrProvider::createEmptyLayer(), but to no avail. So basically I don't understand why offline editing worked before with a GPKG backend. I'd say it is pure luck. Note: I also saw that qgsofflineediting.cpp at line 315 calls offlineLayer->dataProvider()->invalidateConnections() but that method is only implemented in the Spatialite provider. I tried to implement it in the OGR one, but to no avail too.

Basically, for offline editing, we ask QgsOgrProvider to adopt the behaviour it had before qgis#47098, that is use SQLite3 journal_mode = WAL even in read-only mode. Sorry, this "fix" is horrible, but I couldn't get with something better after many hours of investigation. The root cause seems to be that QgsOfflineEditing creates layers using direct OGR API or sqlite3 API, and also uses QgsOgrProvider which has a GDAL dataset reuse mechanism. And all that does not place nicely at all. Maybe the root root cause is that the OGR GPKG driver establishes the list of layers at its opening, and if creates a new one behind its back, which is prone to happen with the dataset caching metchanism, the GDALDataset::GetLayerByName() method doesn't see the newly created layer. With that in mind, I tried to add calls to QgsOgrProviderUtils::invalidateCachedDatasets() but to no avail. Also QgsOgrProviderUtils::invalidateCachedLastModifiedDate() which is used by QgsOgrProvider::createEmptyLayer(), but to no avail. So basically I don't understand why offline editing worked before with a GPKG backend. I'd say it is pure luck. Note: I also saw that qgsofflineediting.cpp at line 315 calls offlineLayer->dataProvider()->invalidateConnections() but that method is only implemented in the Spatialite provider. I tried to implement it in the OGR one, but to no avail too.

Fixes #48012 Basically, for offline editing, we ask QgsOgrProvider to adopt the behaviour it had before #47098, that is use SQLite3 journal_mode = WAL even in read-only mode. Sorry, this "fix" is horrible, but I couldn't get with something better after many hours of investigation. The root cause seems to be that QgsOfflineEditing creates layers using direct OGR API or sqlite3 API, and also uses QgsOgrProvider which has a GDAL dataset reuse mechanism. And all that does not place nicely at all. Maybe the root root cause is that the OGR GPKG driver establishes the list of layers at its opening, and if creates a new one behind its back, which is prone to happen with the dataset caching metchanism, the GDALDataset::GetLayerByName() method doesn't see the newly created layer. With that in mind, I tried to add calls to QgsOgrProviderUtils::invalidateCachedDatasets() but to no avail. Also QgsOgrProviderUtils::invalidateCachedLastModifiedDate() which is used by QgsOgrProvider::createEmptyLayer(), but to no avail. So basically I don't understand why offline editing worked before with a GPKG backend. I'd say it is pure luck. Note: I also saw that qgsofflineediting.cpp at line 315 calls offlineLayer->dataProvider()->invalidateConnections() but that method is only implemented in the Spatialite provider. I tried to implement it in the OGR one, but to no avail too.

agiudiceandrea · 2022-04-07T16:32:14Z

Since this PR causes some regressions (#48003 not yet fixed and #48024 fixed in a not yet released GDAL 3.4.x version), would it be worth to revert its backport (#47301 and cf0af09) at least in the 3.22 LTR branch to prevent the occurrence of such regressions in the upcoming QGIS LTR 3.22.6?

rouault · 2022-04-07T16:51:24Z

Since this PR causes some regressions (#48003 not yet fixed and #48024 fixed in a not yet released GDAL 3.4.x version), would it be worth to revert its backport (#47301 and cf0af09) at least in the 3.22 LTR branch to prevent the occurrence of such regressions in the upcoming QGIS LTR 3.22.6?

hopefully #48119 should fix network issues, but only compile-tested here

agiudiceandrea · 2022-04-07T16:54:30Z

hopefully #48119 should fix network issues, but only compile-tested here

Thanks a lot again! I will try the MingW64 build and report back ASAP.

agiudiceandrea · 2022-04-08T06:47:11Z

I can confirm that #48119 fixes #48003 on Windows 10 (QGIS 3.25.0-Master MingW64 build)

agiudiceandrea · 2022-04-10T10:46:12Z

Hi @rouault, it seems to me that also the issue reported at #48154 ("Adding additional layers to GPKG does not load them correctly") may be related to this PR.

rouault · 2022-04-10T13:37:28Z

Hi @rouault, it seems to me that also the issue reported at #48154 ("Adding additional layers to GPKG does not load them correctly") may be related to this PR.

fix in #48157

agiudiceandrea · 2022-05-23T07:26:49Z

Hi @rouault, I think the issue #48671, which is similar to #48154, could be related to this PR.

curved_polys.gpkg: set journal_mode to delete

e89aab4

github-actions bot added this to the 3.24.0 milestone Jan 31, 2022

rouault force-pushed the avoid_gpkg_wal_on_read_only branch from 95a062a to 99aecd8 Compare January 31, 2022 17:40

rouault added 3 commits January 31, 2022 18:56

curved_poly.gpkg: fix incorrect triggers

7578c41

geopackage.gpkg: set journal_mode to delete

b8ad0fc

rouault force-pushed the avoid_gpkg_wal_on_read_only branch from 99aecd8 to f280714 Compare January 31, 2022 17:57

nyalldawson reviewed Feb 1, 2022

View reviewed changes

src/core/providers/ogr/qgsogrprovider.cpp Show resolved Hide resolved

nyalldawson approved these changes Feb 1, 2022

View reviewed changes

nyalldawson merged commit fe410ec into qgis:master Feb 1, 2022

rouault mentioned this pull request Feb 11, 2022

[queued_ltr_backports] OGR provider: only enable journal_mode = wal when editing. requires GDAL >= 3.4.2 (fixes #23991) #47301

Merged

kannes mentioned this pull request Mar 26, 2022

Opening a Vector Geopackage in QGIS alters its file even when no changes are done by user #23991

Closed

rouault added a commit to rouault/QGIS that referenced this pull request Mar 28, 2022

[GUI] Open vector layer dialog: do not expose GPKG 'NOLOCK' open option

3db73dc

This option is automatically set by the OGR provider. No need for the user to mess with it. Refs qgis#47098 (comment)

rouault mentioned this pull request Mar 28, 2022

[GUI] Open vector layer dialog: do not expose GPKG 'NOLOCK' open option #47992

Merged

agiudiceandrea mentioned this pull request Mar 30, 2022

Geopackage from network location is not a valid or recognized data source #48003

Closed

2 tasks

agiudiceandrea mentioned this pull request Mar 30, 2022

Cannot add GeoPackage layer with hash symbol # in file path #48024

Closed

2 tasks

qgis-bot pushed a commit that referenced this pull request Mar 30, 2022

[GUI] Open vector layer dialog: do not expose GPKG 'NOLOCK' open option

93f9be1

This option is automatically set by the OGR provider. No need for the user to mess with it. Refs #47098 (comment)

qgis-bot pushed a commit that referenced this pull request Mar 30, 2022

[GUI] Open vector layer dialog: do not expose GPKG 'NOLOCK' open option

d3a77b6

This option is automatically set by the OGR provider. No need for the user to mess with it. Refs #47098 (comment)

nirvn pushed a commit that referenced this pull request Mar 31, 2022

[GUI] Open vector layer dialog: do not expose GPKG 'NOLOCK' open option

afe05ba

This option is automatically set by the OGR provider. No need for the user to mess with it. Refs #47098 (comment)

nirvn pushed a commit that referenced this pull request Mar 31, 2022

[GUI] Open vector layer dialog: do not expose GPKG 'NOLOCK' open option

089725b

This option is automatically set by the OGR provider. No need for the user to mess with it. Refs #47098 (comment)

rouault mentioned this pull request Apr 2, 2022

[Offline editing] Unbreak TestQgsOfflineEditing with GDAL >= 3.4.2 #48059

Merged

agiudiceandrea mentioned this pull request Apr 10, 2022

Adding additional layers to GPKG does not load them correctly #48154

Closed

2 tasks

agiudiceandrea mentioned this pull request May 23, 2022

New GeoPackage layer cannot be loaded (QGIS thinks it's invalid but it isn't really) #48671

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OGR provider: only enable journal_mode = wal when editing. requires GDAL >= 3.4.2 (fixes #23991) #47098

OGR provider: only enable journal_mode = wal when editing. requires GDAL >= 3.4.2 (fixes #23991) #47098

rouault commented Jan 31, 2022

nirvn commented Jan 31, 2022

gioman commented Jan 31, 2022

kannes commented Feb 1, 2022

florisvdh commented Feb 2, 2022

gdt commented Feb 2, 2022

rouault commented Feb 2, 2022

agiudiceandrea commented Mar 24, 2022

rouault commented Mar 24, 2022

agiudiceandrea commented Mar 24, 2022 •

edited

rouault commented Mar 24, 2022

agiudiceandrea commented Mar 24, 2022 •

edited

agiudiceandrea commented Mar 28, 2022 •

edited

rduivenvoorde commented Mar 28, 2022

agiudiceandrea commented Mar 28, 2022

rouault commented Mar 28, 2022

agiudiceandrea commented Mar 31, 2022

agiudiceandrea commented Apr 7, 2022

rouault commented Apr 7, 2022

agiudiceandrea commented Apr 7, 2022

agiudiceandrea commented Apr 8, 2022

agiudiceandrea commented Apr 10, 2022

rouault commented Apr 10, 2022

agiudiceandrea commented May 23, 2022

OGR provider: only enable journal_mode = wal when editing. requires GDAL >= 3.4.2 (fixes #23991) #47098

OGR provider: only enable journal_mode = wal when editing. requires GDAL >= 3.4.2 (fixes #23991) #47098

Conversation

rouault commented Jan 31, 2022

nirvn commented Jan 31, 2022

gioman commented Jan 31, 2022

kannes commented Feb 1, 2022

florisvdh commented Feb 2, 2022

gdt commented Feb 2, 2022

rouault commented Feb 2, 2022

agiudiceandrea commented Mar 24, 2022

rouault commented Mar 24, 2022

agiudiceandrea commented Mar 24, 2022 • edited

rouault commented Mar 24, 2022

agiudiceandrea commented Mar 24, 2022 • edited

agiudiceandrea commented Mar 28, 2022 • edited

rduivenvoorde commented Mar 28, 2022

agiudiceandrea commented Mar 28, 2022

rouault commented Mar 28, 2022

agiudiceandrea commented Mar 31, 2022

agiudiceandrea commented Apr 7, 2022

rouault commented Apr 7, 2022

agiudiceandrea commented Apr 7, 2022

agiudiceandrea commented Apr 8, 2022

agiudiceandrea commented Apr 10, 2022

rouault commented Apr 10, 2022

agiudiceandrea commented May 23, 2022

agiudiceandrea commented Mar 24, 2022 •

edited

agiudiceandrea commented Mar 24, 2022 •

edited

agiudiceandrea commented Mar 28, 2022 •

edited