On 2025/05/24 5:41, Kevin K Biju wrote:
Hi,
While creating a logical replication slot, we wait for older transactions to complete to
reach a "consistent point", which can take a while on busy databases. If we're
creating a slot on a primary instance, it's pretty clear that we're waiting on a
transaction:
postgres=# SELECT pid,wait_event_type,wait_event,state,query FROM
pg_stat_activity WHERE pid=804;
pid | wait_event_type | wait_event | state |
query
-----+-----------------+---------------+--------+----------------------------------------------------------------
804 | Lock | transactionid | active | SELECT
pg_create_logical_replication_slot('wow1', 'pgoutput');
(1 row)
postgres=# SELECT locktype,transactionid,pid,mode FROM pg_locks WHERE pid=804
AND granted='f';
locktype | transactionid | pid | mode
---------------+---------------+-----+-----------
transactionid | 6077 | 804 | ShareLock
However, creating a slot on a hot standby behaves very differently when blocked
on a transaction. Querying pg_stat_activity doesn't give us any indication on
what the issue is:
postgres=# SELECT pg_is_in_recovery();
pg_is_in_recovery
-------------------
t
(1 row)
postgres=# SELECT pid,wait_event_type,wait_event,state,query FROM
pg_stat_activity WHERE pid=5074;
pid | wait_event_type | wait_event | state |
query
------+-----------------+------------+--------+----------------------------------------------------------------
5074 | | | active | SELECT
pg_create_logical_replication_slot('wow1', 'pgoutput');
And more importantly, a backend in this state cannot be terminated via the
usual methods and sometimes requires a server restart.
postgres=# SELECT pg_cancel_backend(5074), pg_terminate_backend(5074);
pg_cancel_backend | pg_terminate_backend
-------------------+----------------------
t | t
(1 row)
postgres=# SELECT pid,wait_event_type,wait_event,state,query FROM
pg_stat_activity WHERE pid=5074;
pid | wait_event_type | wait_event | state |
query
------+-----------------+------------+--------+----------------------------------------------------------------
5074 | | | active | SELECT
pg_create_logical_replication_slot('wow1', 'pgoutput');
I've taken a look at the code that "awaits" on transactions and the function of
interest is lmgr.c/XactLockTableWait. On a primary, it is able to acquire a ShareLock on
the xid and the lock manager does a good job on making this wait efficient as well as
visible externally. However, on a standby, it cannot wait on the xid since it is not
running the transaction. However, it knows the transaction is still running from
KnownAssignedXids, and then ends up on this codepath:
*
* Some uses of this function don't involve tuple visibility -- such
* as when building snapshots for logical decoding. It is possible to
* see a transaction in ProcArray before it registers itself in the
* locktable. The topmost transaction in that case is the same xid,
* so we try again after a short sleep. (Don't sleep the first time
* through, to avoid slowing down the normal case.)
*/
if (!first)
pg_usleep(1000L);
The attached comment suggests that this sleep was only meant to be hit a few
times while we wait for the lock to appear so we can wait on it. However, in
the hot standby case, this ends up becoming a polling loop since the lock will
never appear. There is no interrupt processing in this loop either and so the
only way out is to wait for the transaction on the primary to complete.
I agree with your analysis. It makes sense to me.
Attached is a patch that adds CHECK_FOR_INTERRUPTS before the sleep in
XactLockTableWait and ConditionalXactLockTableWait. This allows backends
waiting on a transaction during slot creation on a standby to be interrupted.
+1
It would also be nice if there was a way for users to tell what we're waiting
on (maybe a different wait event?) but I'd like input on that.
Just an idea: how about calling pgstat_report_wait_start() and _end() around
pg_usleep(), similar to what WaitExceedsMaxStandbyDelay() does?
Since this is more of an improvement than a bug fix, I think
it would be better to make it as a separate patch from the CFI addition.
Also, would waiting only 1ms per loop cycle be too aggressive? If so, maybe
we could increase the sleep time gradually during recovery (but not beyond
1 second), again similar to WaitExceedsMaxStandbyDelay().
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
