Dev-iL commented on code in PR #76976:
URL: https://github.com/apache/airflow/pull/76976#discussion_r4550390727
##########
airflow-core/src/airflow/utils/db.py:
##########
@@ -1642,10 +1647,12 @@ def _create_global_lock_postgresql(
text("SELECT set_config('lock_timeout', :timeout, false)"),
{"timeout": str(lock_timeout)},
)
- conn.execute(text("SELECT pg_advisory_lock(:id)"), {"id":
lock.value})
else:
conn.execute(text("SET LOCK_TIMEOUT to :timeout"), {"timeout":
lock_timeout})
- conn.execute(text("SELECT pg_advisory_lock(:id)"), {"id":
lock.value})
+ # No bound parameter: under psycopg3 a parameterized statement keeps
its snapshot while
+ # the connection sits idle in transaction, and CREATE INDEX
CONCURRENTLY run by a
+ # migration in this process would wait on it forever.
Review Comment:
Small precision point on this comment. The snapshot is held by the *most
recent* statement on the connection, not by any parameterized statement that
ran earlier. Measured on psycopg 3.3.8: this inlined lock followed by any
parameterized statement (for example `SELECT :x`) holds `backend_xmin` and
hangs `CREATE INDEX CONCURRENTLY` again, while a parameterless statement after
a parameterized lock releases it.
That follows from the protocol. psycopg only uses the simple query protocol
when there are no parameters (`_execute_send` in `_cursor_base.py`). A
parameterized statement goes through the extended protocol, and its unnamed
portal keeps the snapshot after the statement finishes. The [PostgreSQL
protocol
docs](https://www.postgresql.org/docs/17/protocol-flow.html#PROTOCOL-FLOW-EXT-QUERY)
say: "An unnamed portal is destroyed at the end of the transaction, or as soon
as the next Bind statement specifying the unnamed portal as destination is
issued. (Note that a simple Query message also destroys the unnamed portal.)"
So the rule is "the last statement run on the lock connection before the
`yield` must have no bound parameters". Worth saying so, otherwise a later edit
that adds a parameterized statement after the lock silently brings the hang
back. The new test would catch it, but only on Postgres with psycopg 3.
```suggestion
# The last statement run on this connection before the yield must
have no bound parameter:
# under psycopg3 a parameterized statement keeps its snapshot while
the connection sits idle
# in transaction, and CREATE INDEX CONCURRENTLY run by a migration
in this process would
# wait on it forever.
```
---
Drafted-by: Claude Code, reviewed by @Dev-iL before posting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
