Re: [HACKERS] make async slave to wait for lsn to be replayed

Euler Taveira <euler@eulerto.com>

From: "Euler Taveira" <euler@eulerto.com>
To: "Alexander Korotkov" <aekorotkov@gmail.com>, "Thomas Munro" <thomas.munro@gmail.com>
Cc: "Kartyshov Ivan" <i.kartyshov@postgrespro.ru>, "Bharath Rupireddy" <bharath.rupireddyforpostgres@gmail.com>, "Peter Eisentraut" <peter@eisentraut.org>, "Amit Kapila" <amit.kapila16@gmail.com>, "Dilip Kumar" <dilipbalaut@gmail.com>, "Peter Smith" <smithpb2250@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-03-28T23:37:54Z
Lists: pgsql-hackers
On Thu, Mar 28, 2024, at 9:39 AM, Alexander Korotkov wrote:
> Fixed along with other issues spotted by Alexander Lakhin.

[I didn't read the whole thread. I'm sorry if I missed something ...]

You renamed the function in a previous version but let me suggest another one:
pg_wal_replay_wait. It uses the same pattern as the other recovery control
functions [1]. I think "for" doesn't add much for the function name and "lsn" is
used in functions that return an LSN (that's not the case here).

postgres=# \df pg_wal_replay*
List of functions
-[ RECORD 1 ]-------+---------------------
Schema              | pg_catalog
Name                | pg_wal_replay_pause
Result data type    | void
Argument data types | 
Type                | func
-[ RECORD 2 ]-------+---------------------
Schema              | pg_catalog
Name                | pg_wal_replay_resume
Result data type    | void
Argument data types | 
Type                | func

Regarding the arguments, I think the timeout should be bigint. There is at least
another function that implements a timeout that uses bigint. 

postgres=# \df pg_terminate_backend
List of functions
-[ RECORD 1 ]-------+--------------------------------------
Schema              | pg_catalog
Name                | pg_terminate_backend
Result data type    | boolean
Argument data types | pid integer, timeout bigint DEFAULT 0
Type                | func

I also suggests that the timeout unit should be milliseconds, hence, using
bigint is perfectly fine for the timeout argument.

+       <para>
+        Throws an ERROR if the target <acronym>lsn</acronym> was not replayed
+        on standby within given timeout.  Parameter <parameter>timeout</parameter>
+        is the time in seconds to wait for the <parameter>target_lsn</parameter>
+        replay. When <parameter>timeout</parameter> value equals to zero no
+        timeout is applied.
+       </para></entry>


[1] https://www.postgresql.org/docs/current/functions-admin.html#FUNCTIONS-RECOVERY-CONTROL


--
Euler Taveira
EDB   https://www.enterprisedb.com/

Commits

  1. Ensure standby promotion point in 043_wal_replay_wait.pl

  2. Minor cleanup related to pg_wal_replay_wait() procedure

  3. Adjust pg_wal_replay_wait() procedure behavior on promoted standby

  4. pg_wal_replay_wait(): Fix typo in the doc

  5. Implement pg_wal_replay_wait() stored procedure

  6. Revert: Implement pg_wal_replay_wait() stored procedure

  7. Call WaitLSNCleanup() in AbortTransaction()

  8. Clarify what is protected by WaitLSNLock

  9. Use an LWLock instead of a spinlock in waitlsn.c

  10. Use the pairing heap instead of a flat array for LSN replay waiters

  11. Minor improvements for waitlsn.c

  12. Make the order of the header file includes consistent