Revise index lock mode in spatial_index_read_extent

[Algunenano]

From IRC, use andehhh has reported a locking problem related with an external process having an AccessShareLock to the spatial index and a call to st_estimatedextent.

As st_estimatedextent is using AccessExclusiveLock it will block until the other function finishes even though it's only using it for reading.

We could revise whether that level of exclusive access to the index is really needed or we can make do with less "exclusivity".

From [1]:

Index access methods must handle concurrent updates of the index by multiple processes. The core PostgreSQL system obtains AccessShareLock on the index during an index scan, and RowExclusiveLock when updating the index (including plain VACUUM). Since these lock types do not conflict, the access method is responsible for handling any fine-grained locking it might need. An exclusive lock on the index as a whole will be taken only during index creation, destruction, or REINDEX.

I understand that the operation we are doing in spatial_index_read_extent is equivalent to an index scan but I don't think the perf gain it's worth the added complexity of dealing with concurrent modifications. We could avoid that using "just" a ExclusiveLock, which allows concurrent read locks but blocks any modifications.

Am I missing something?

Some refs:

[1] ​https://www.postgresql.org/docs/11/index-locking.html

[2] ​https://www.postgresql.org/docs/11/index-scanning.html

[3] ​https://www.postgresql.org/docs/11/explicit-locking.html

[andehhh]

I would even go further than classifying this ticket just as "enhancement". From a users' point of view this behavior is not desirable. When doing processing on a table using PostGIS 2.5.x the AccessExclusiveLock prevents any other user to even view the data in e.g. QGIS. Also when it comes to development tasks where many trial runs for processes are necessary it would always have effects on the front end as well.

cheers

[pramsey]

I think changing the lock mode makes sense, and I think backporting also makes sense.

[pramsey]

In 17167:

Use AccessShareLock in spatial_index_read_extent
References #4261

[pramsey]

In 17168:

Use AccessShareLock in spatial_index_read_extent
References #4261

[andehhh]

Great! Thank you!