Hi All, In [1] we discussed many issues related to SQL/PGQ. There were many locking related issues reported. Collecting them together with respective solutions in one thread for future reference. I this the issues and the solutions are straight forward. But having them together will serve as a good checklist for the next round.
1. ALTER PROPERTY GRAPH did not exclude concurrent graph readers. AlterPropGraph() used ShareRowExclusiveLock which serializes graph alterations but does not conflict with the AccessShareLock held by GRAPH_TABLE queries. GRAPH_TABLE rewriter reads different parts of the property graph definition at different times. A concurrent modification of the property graph could cause different reads to be inconsistent with each other. Solution is to use AccessExclusiveLock in AlterPropGraph(). 2. DROP CASCADE could modify graph components without locking their parent graph. Locking a component object, being dropped as part of the DROP CASCADE, alone does not conflict with readers or ALTER PROPERTY GRAPH, which lock the graph relation. Thus modifications of property graphs caused because of a CASCADed DROP may render a property graph inconsistent especially in the presence of a concurrent AlterPropGraph(). Also the concurrent readers may read inconsistent property graph definition. Solution is to extend AcquireDeletionLock() to find and take AccessExclusiveLock (like AlterPropGraph()) on the containing graph for a property graph component. graph-before-component order. When performDeletion() encounters a property graph component it should invalidate all the caches that reference the property graph. This has been briefly mentioned in [2] as well. 3. insert_property_records() read element-table attributes before locking that table. ALTER PROPERTY GRAPH ADD LABEL and ADD PROPERTIES can reach it without an element-table lock; PROPERTIES ALL may scan pg_attribute() without a lock on the element table. Solution, call table_open(pgerelid, AccessShareLock) at the beginning of the function instead of later. 4. Read-only catalog scans used unexplained RowShareLock modes. is_property_associated_with_label() and get_element_property_expr() used RowShareLock on pg_propgraph_element_label; insert_property_records() used it on pg_attribute. RowShareLock does not conflict with the RowExclusiveLock used for catalog writes, so it does not provide the required protection against concurrent metadata changes. The scans should instead use AccessShareLock. The relevant graph or element relation lock provides that protection. This is lock-mode cleanup, separate from the missing element-table lock above. 5. Document in replace_property_refs()'s prologue that its callers have to take a lock on the property graph. Better to confirm that assumption with a Assert(CheckRelationOidLockedByMe(propgraphid, AccessShareLock, true)) there. 6. Rewriter element-table metadata access before locking. build_edge_vertex_link_quals() calls get_atttypetypmodcoll() before the locking the corresponding element table. We did not find a visible hazard, but it's better to lock the element table before accessing its metadata. 7. Concurrent DDL needed coverage while a query was actually being rewritten. Add isolation tests for DDL vs query, AlterPropGraph vs DROP CASCADE. [1] https://www.postgresql.org/message-id/dqa5mstx5mna3i7s23pdwl4m6bek7gqsgfccef44wjpswizufi@3aa6vzri3cat [2] https://www.postgresql.org/message-id/caexhw5suht9mmjsegakmx8z8nykt0dxegw_q4f7_afvdtrk...@mail.gmail.com -- Best Wishes, Ashutosh Bapat
