pgsql: Implement pg_wal_replay_wait() stored procedure

On 02.08.24 20:22, Alexander Korotkov wrote:

Implement pg_wal_replay_wait() stored procedure

Why is this under src/backend/command/? Wouldn't it belong under
src/backend/utils/adt/?

On Fri, Aug 30, 2024 at 10:42 PM Peter Eisentraut <peter@eisentraut.org> wrote:

On 02.08.24 20:22, Alexander Korotkov wrote:

Implement pg_wal_replay_wait() stored procedure

Why is this under src/backend/command/? Wouldn't it belong under
src/backend/utils/adt/?

This path hasn't changes since the patch revision when it was a
utility command. I agree that this doesn't look like proper path for
stored procedure. But I don't think src/backend/utils/adt is
appropriate path either, because it's not really about data type.
pg_wal_replay_wait() looks a good neighbor for
pg_wal_replay_pause()/pg_wal_replay_resume() and other WAL-related
functions. So, what about moving it to src/backend/access/transam?

------
Regards,
Alexander Korotkov
Supabase

On Sun, Sep 01, 2024 at 10:35:27PM +0300, Alexander Korotkov wrote:

This path hasn't changes since the patch revision when it was a
utility command. I agree that this doesn't look like proper path for
stored procedure. But I don't think src/backend/utils/adt is
appropriate path either, because it's not really about data type.
pg_wal_replay_wait() looks a good neighbor for
pg_wal_replay_pause()/pg_wal_replay_resume() and other WAL-related
functions. So, what about moving it to src/backend/access/transam?

Moving the new function to xlogfuncs.c while publishing
WaitForLSNReplay() makes sense to me.
--
Michael

On Mon, Sep 2, 2024 at 2:28 AM Michael Paquier <michael@paquier.xyz> wrote:

On Sun, Sep 01, 2024 at 10:35:27PM +0300, Alexander Korotkov wrote:

This path hasn't changes since the patch revision when it was a
utility command. I agree that this doesn't look like proper path for
stored procedure. But I don't think src/backend/utils/adt is
appropriate path either, because it's not really about data type.
pg_wal_replay_wait() looks a good neighbor for
pg_wal_replay_pause()/pg_wal_replay_resume() and other WAL-related
functions. So, what about moving it to src/backend/access/transam?

Moving the new function to xlogfuncs.c while publishing
WaitForLSNReplay() makes sense to me.

Thank you for proposal. I like this.

Could you, please, check the attached patch?

------
Regards,
Alexander Korotkov
Supabase

On Mon, Sep 02, 2024 at 02:55:50AM +0300, Alexander Korotkov wrote:

Could you, please, check the attached patch?

The patch moving the code looks correct at quick glance.

Now, I've been staring at this line, wondering why this is required
while WaitForLSNReplay() does not return any status:
+ (void) WaitForLSNReplay(target_lsn, timeout);

And this makes me question whether you have the right semantics
regarding the SQL function itself. Could it be more useful for
WaitForLSNReplay() to report an enum state that tells you the reason
why a wait may not have happened with a text or a tuple returned by
the function? There are quite a few reasons why a wait may or may not
happen:
- Recovery has ended, target LSN has been reached.
- We're not in recovery anymore. Is an error the most useful thing
here actually?
- Timeout may have been reached, where an error is also generated.
ERRCODE_QUERY_CANCELED is not a correct error state.
- Perhaps more of these in the future?

My point is: this is a feature that can be used for monitoring as far
as I know, still it does not stick as a feature that could be most
useful for such applications. Wouldn't it be more useful for client
applications to deal with a state returned by the SQL function rather
than having to parse error strings to know what happened? What is
returned by pg_wal_replay_wal() reflects on the design of
WaitForLSNReplay() itself.
--
Michael

Hi, Michael!

On Mon, Sep 2, 2024 at 3:25 AM Michael Paquier <michael@paquier.xyz> wrote:

On Mon, Sep 02, 2024 at 02:55:50AM +0300, Alexander Korotkov wrote:

Could you, please, check the attached patch?

The patch moving the code looks correct at quick glance.

Thank you for your feedback.

Now, I've been staring at this line, wondering why this is required
while WaitForLSNReplay() does not return any status:
+ (void) WaitForLSNReplay(target_lsn, timeout);

And this makes me question whether you have the right semantics
regarding the SQL function itself. Could it be more useful for
WaitForLSNReplay() to report an enum state that tells you the reason
why a wait may not have happened with a text or a tuple returned by
the function? There are quite a few reasons why a wait may or may not
happen:
- Recovery has ended, target LSN has been reached.
- We're not in recovery anymore. Is an error the most useful thing
here actually?
- Timeout may have been reached, where an error is also generated.
ERRCODE_QUERY_CANCELED is not a correct error state.
- Perhaps more of these in the future?

My point is: this is a feature that can be used for monitoring as far
as I know, still it does not stick as a feature that could be most
useful for such applications. Wouldn't it be more useful for client
applications to deal with a state returned by the SQL function rather
than having to parse error strings to know what happened? What is
returned by pg_wal_replay_wal() reflects on the design of
WaitForLSNReplay() itself.

By design, pg_wal_replay_wal() should be followed up with read-only
query to standby. The read-only query gets guarantee that it's
executed after the replay of LSN specified as pg_wal_replay_wal()
design. Returning the status from pg_wal_replay_wal() would require
additional cycle of its processing. But one of the main objectives of
this feature was reducing roundtrips during waiting for LSN.

On the other hand, I see that returning status could make sense for
certain use cases. I think I could write two patches to provide that.
1. Make WaitForLSNReplay() return status, and make pg_wal_replay_wal()
be responsible for throwing all the errors.
2. New procedure pg_wal_replay_wal_status() (or some better name?),
which returns status to the user instead of throwing errors.

If no objections, I will push the patch moving code then go ahead
writing the two patches above.

------
Regards,
Alexander Korotkov
Supabase

On Tue, Sep 3, 2024 at 4:07 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:

If no objections, I will push the patch moving code then go ahead
writing the two patches above.

The patch for code movement missed couple of includes. Revised
version is attached.

------
Regards,
Alexander Korotkov
Supabase

On Tue, Sep 3, 2024 at 4:07 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:

On the other hand, I see that returning status could make sense for
certain use cases. I think I could write two patches to provide that.
1. Make WaitForLSNReplay() return status, and make pg_wal_replay_wal()
be responsible for throwing all the errors.
2. New procedure pg_wal_replay_wal_status() (or some better name?),
which returns status to the user instead of throwing errors.

If no objections, I will push the patch moving code then go ahead
writing the two patches above.

I attempted to implement the patchset as promised. The 0001 is easy
and straighforward. The 0002 became tricky. Procedures can't return
values. They can have OUTPUT parameters instead. However, even for
output parameters you need to pass something in, and that couldn't be
a default value. Additional difficulty is that having OUTPUT
parameters seem to hold additional snapshot and prevents our
snapshot-releasing trick from working....

smagen@postgres=# CALL pg_wal_replay_wait('0/0'::pg_lsn);
CALL
Time: 2.061 ms
smagen@postgres=# CALL pg_wal_replay_wait_status(NULL, '0/0'::pg_lsn);
ERROR: pg_wal_replay_wait_status() must be only called without an
active or registered snapshot
DETAIL: Make sure pg_wal_replay_wait_status() isn't called within a
transaction with an isolation level higher than READ COMMITTED,
another procedure, or a function.
Time: 1.175 ms

I'm thinking about following solution:
1. pg_wal_replay_wait_no_error() procedure, which doesn't return
anything but throws no errors.
2. Make pg_wal_replay_wait()/pg_wal_replay_wait_no_error() save
waiting result status to the local variable.
3. New function pg_wal_replay_wal_get_status() displaying the result
status of the last pg_wal_replay_wait()/pg_wal_replay_wait_no_error()
CALL.

Then one could do.
CALL pg_wal_replay_wait_no_error(...);
SELECT pg_wal_replay_wal_get_status();

Probably looks a bit excessive, but probably the best we can do.

------
Regards,
Alexander Korotkov
Supabase

On Thu, Sep 19, 2024 at 3:47 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:

On Tue, Sep 3, 2024 at 4:07 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:

On the other hand, I see that returning status could make sense for
certain use cases. I think I could write two patches to provide that.
1. Make WaitForLSNReplay() return status, and make pg_wal_replay_wal()
be responsible for throwing all the errors.
2. New procedure pg_wal_replay_wal_status() (or some better name?),
which returns status to the user instead of throwing errors.

If no objections, I will push the patch moving code then go ahead
writing the two patches above.

I attempted to implement the patchset as promised. The 0001 is easy
and straighforward. The 0002 became tricky. Procedures can't return
values. They can have OUTPUT parameters instead. However, even for
output parameters you need to pass something in, and that couldn't be
a default value. Additional difficulty is that having OUTPUT
parameters seem to hold additional snapshot and prevents our
snapshot-releasing trick from working....

smagen@postgres=# CALL pg_wal_replay_wait('0/0'::pg_lsn);
CALL
Time: 2.061 ms
smagen@postgres=# CALL pg_wal_replay_wait_status(NULL, '0/0'::pg_lsn);
ERROR: pg_wal_replay_wait_status() must be only called without an
active or registered snapshot
DETAIL: Make sure pg_wal_replay_wait_status() isn't called within a
transaction with an isolation level higher than READ COMMITTED,
another procedure, or a function.
Time: 1.175 ms

I'm thinking about following solution:
1. pg_wal_replay_wait_no_error() procedure, which doesn't return
anything but throws no errors.
2. Make pg_wal_replay_wait()/pg_wal_replay_wait_no_error() save
waiting result status to the local variable.
3. New function pg_wal_replay_wal_get_status() displaying the result
status of the last pg_wal_replay_wait()/pg_wal_replay_wait_no_error()
CALL.

Then one could do.
CALL pg_wal_replay_wait_no_error(...);
SELECT pg_wal_replay_wal_get_status();

Probably looks a bit excessive, but probably the best we can do.

Please, check the attached patchset for implementation of proposed approach.

------
Regards,
Alexander Korotkov
Supabase

Hi Alexander,

On Fri, Aug 02, 2024 at 06:22:21PM +0000, Alexander Korotkov wrote:

Implement pg_wal_replay_wait() stored procedure

pg_wal_replay_wait() is to be used on standby and specifies waiting for
the specific WAL location to be replayed. This option is useful when
the user makes some data changes on primary and needs a guarantee to see
these changes are on standby.

The queue of waiters is stored in the shared memory as an LSN-ordered pairing
heap, where the waiter with the nearest LSN stays on the top. During
the replay of WAL, waiters whose LSNs have already been replayed are deleted
from the shared memory pairing heap and woken up by setting their latches.

pg_wal_replay_wait() needs to wait without any snapshot held. Otherwise,
the snapshot could prevent the replay of WAL records, implying a kind of
self-deadlock. This is why it is only possible to implement
pg_wal_replay_wait() as a procedure working without an active snapshot,
not a function.

Catversion is bumped.

I've spotted that this patch uses an OID that should not be used
during the development cycle:
+{ oid => '111',
+  descr => 'wait for the target LSN to be replayed on standby with an optional timeout', 

Please use something in the 8000-9999 range, as required by
98eab30b93d5.

Thanks,
--
Michael

On Thu, Sep 26, 2024 at 11:19 AM Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Aug 02, 2024 at 06:22:21PM +0000, Alexander Korotkov wrote:

Implement pg_wal_replay_wait() stored procedure

pg_wal_replay_wait() is to be used on standby and specifies waiting for
the specific WAL location to be replayed. This option is useful when
the user makes some data changes on primary and needs a guarantee to see
these changes are on standby.

The queue of waiters is stored in the shared memory as an LSN-ordered pairing
heap, where the waiter with the nearest LSN stays on the top. During
the replay of WAL, waiters whose LSNs have already been replayed are deleted
from the shared memory pairing heap and woken up by setting their latches.

pg_wal_replay_wait() needs to wait without any snapshot held. Otherwise,
the snapshot could prevent the replay of WAL records, implying a kind of
self-deadlock. This is why it is only possible to implement
pg_wal_replay_wait() as a procedure working without an active snapshot,
not a function.

Catversion is bumped.

I've spotted that this patch uses an OID that should not be used
during the development cycle:
+{ oid => '111',
+  descr => 'wait for the target LSN to be replayed on standby with an optional timeout',

Please use something in the 8000-9999 range, as required by
98eab30b93d5.

Fixed, sorry for messing this up.
I would appreciate if you have time to look at [0] to check if it
meets your expectations.

Links.
1. /messages/by-id/CAPpHfdsw9oq62Fvt65JApHJf1auUirdGJV7=nRyVnDL3M8z5xA@mail.gmail.com

------
Regards,
Alexander Korotkov
Supabase

On Thu, Sep 26, 2024 at 06:41:18PM +0300, Alexander Korotkov wrote:

On Thu, Sep 26, 2024 at 11:19 AM Michael Paquier <michael@paquier.xyz> wrote:

Please use something in the 8000-9999 range, as required by
98eab30b93d5.

Fixed, sorry for messing this up.

Thanks for taking care of that.

I would appreciate if you have time to look at [0] to check if it
meets your expectations.

Links.
1. /messages/by-id/CAPpHfdsw9oq62Fvt65JApHJf1auUirdGJV7=nRyVnDL3M8z5xA@mail.gmail.com

I am aware of this one, sorry for the delay. It's on my pile of
things to look at, but I have not been able to get back to it.
--
Michael

On Fri, Sep 20, 2024 at 03:00:20PM +0300, Alexander Korotkov wrote:

Please, check the attached patchset for implementation of proposed approach.

0001 looks like it requires an indentation in its .h diffs.

+typedef enum
+{
+	WaitLSNResultSuccess, /* Target LSN is reached */
+	WaitLSNResultTimeout, /* Timeout occured */

Perhaps use WAIT_LSN_RESULT_SUCCESS, etc, rather than camel casing.

+ * Results statuses for WaitForLSNReplay().

Too much plural here.

What you are doing with WaitForLSNReplay() sounds kind of right.

rc = WaitLatch(MyLatch, wake_events, delay_ms,
WAIT_EVENT_WAIT_FOR_WAL_REPLAY);

Question about this existing bit in waitlsn.c. Shouldn't this issue a
FATAL if rc reports a WL_POSTMASTER_DEATH? Or am I missing an
intention here. That was already the case before this patch set.

pg_wal_replay_wait() is new to v18, meaning that we still have some
time to agree on its final shape for this release cycle. This
discussion shares similarities with the recent exercise done in
f593c5517d14, and I think that we should be more consistent between
both to not repeat the same mistakes as in the past, even if this case
if more complex because we have more reasons behind why a wait could
not have happened.

I would suggest to keep things simple and have one single function
rather than introduce two more pg_proc entries with slight differences
in their error reporting, making the original function return a text
about the reason of the failure when waiting (be it a timeout,
success, promotion or outside recovery).

FWIW, I am confused regarding the need for WaitLSNResultNotInRecovery
and WaitLSNResultPromotedConcurrently in the error states. On a code
basis, they check the same thing: RecoveryInProgress(). I'd suggest
to cut one of them. This also points to the fact that
WaitForLSNReplay() has some duplication that's not required. We could
have less GetXLogReplayRecPtr() calls and do all the checks in the for
loop. The current structure of the code in waitlsn.c is more complex
than it could be.
--
Michael

On Fri, Sep 27, 2024 at 01:35:23PM +0900, Michael Paquier wrote:

I would suggest to keep things simple and have one single function
rather than introduce two more pg_proc entries with slight differences
in their error reporting, making the original function return a text
about the reason of the failure when waiting (be it a timeout,
success, promotion or outside recovery).

More to the point here. I am not sure what's the benefit of having a
procedure, while we have been using SQL functions for similar purposes
in xlogfuncs.c for years. And what you have here can also be coupled
with more complex logic in SQL queries.
--
Michael

On Fri, Sep 27, 2024 at 7:57 AM Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Sep 27, 2024 at 01:35:23PM +0900, Michael Paquier wrote:

I would suggest to keep things simple and have one single function
rather than introduce two more pg_proc entries with slight differences
in their error reporting, making the original function return a text
about the reason of the failure when waiting (be it a timeout,
success, promotion or outside recovery).

More to the point here. I am not sure what's the benefit of having a
procedure, while we have been using SQL functions for similar purposes
in xlogfuncs.c for years. And what you have here can also be coupled
with more complex logic in SQL queries.

As I multiple times pointed in the thread [1] [2], this couldn't be
done in SQL function. SQL-function should run within snapshot, which
could prevent WAL from being replayed. In turn, depending on timeout
settings that could lead to hidden deadlock (function waits for LSN to
be replayed, replay process wait snapshot to be released) or an error.

In the stored procedure, we're releasing active and catalog snapshots
(similarly to VACUUM statement). Waiting without holding a snapshots
allows us to workaround the problem described above. We can't do this
in the SQL function, because that would leave the rest of SQL query
without a snapshot.

Links.
1. /messages/by-id/CAPpHfduBSN8j5j5Ynn5x=ThD=8ypNd53D608VXGweBsPzxPvqA@mail.gmail.com
2. /messages/by-id/CAPpHfdtiGgn0iS1KbW2HTam-1+oK+vhXZDAcnX9hKaA7Oe=F-A@mail.gmail.com

------
Regards,
Alexander Korotkov
Supabase

Hi!

Thank you for your review.

On Fri, Sep 27, 2024 at 7:35 AM Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Sep 20, 2024 at 03:00:20PM +0300, Alexander Korotkov wrote:

Please, check the attached patchset for implementation of proposed approach.

0001 looks like it requires an indentation in its .h diffs.

+typedef enum
+{
+       WaitLSNResultSuccess, /* Target LSN is reached */
+       WaitLSNResultTimeout, /* Timeout occured */

Perhaps use WAIT_LSN_RESULT_SUCCESS, etc, rather than camel casing.

+ * Results statuses for WaitForLSNReplay().

Too much plural here.

What you are doing with WaitForLSNReplay() sounds kind of right.

rc = WaitLatch(MyLatch, wake_events, delay_ms,
WAIT_EVENT_WAIT_FOR_WAL_REPLAY);

Thank you, fixed in the attached patchset.

Question about this existing bit in waitlsn.c. Shouldn't this issue a
FATAL if rc reports a WL_POSTMASTER_DEATH? Or am I missing an
intention here. That was already the case before this patch set.

Fixed in the separate patch.

pg_wal_replay_wait() is new to v18, meaning that we still have some
time to agree on its final shape for this release cycle. This
discussion shares similarities with the recent exercise done in
f593c5517d14, and I think that we should be more consistent between
both to not repeat the same mistakes as in the past, even if this case
if more complex because we have more reasons behind why a wait could
not have happened.

I would suggest to keep things simple and have one single function
rather than introduce two more pg_proc entries with slight differences
in their error reporting, making the original function return a text
about the reason of the failure when waiting (be it a timeout,
success, promotion or outside recovery).

I also like to keep things simple. Keeping this as a single function
is not possible due to the reasons I described in [1]. However, it's
possible to fit into one stored procedure. I made 'no_error' as an
argument for the pg_wal_replay_wait() procedure. Done so in the
attached patchset.

FWIW, I am confused regarding the need for WaitLSNResultNotInRecovery
and WaitLSNResultPromotedConcurrently in the error states. On a code
basis, they check the same thing: RecoveryInProgress(). I'd suggest
to cut one of them.

Agreed. I initially intended to distinguish situations when the user
mistakenly calls pg_wal_replay_wait() on replication leader and when
concurrent promotion happens. However, given that the promotion could
happen after the user issued the query and before waiting starts, it
doesn't make much sense.

This also points to the fact that
WaitForLSNReplay() has some duplication that's not required. We could
have less GetXLogReplayRecPtr() calls and do all the checks in the for
loop. The current structure of the code in waitlsn.c is more complex
than it could be.

Not sure about this. I'd like to keep the fast-path check before we
call addLSNWaiter().

Links.
1. /messages/by-id/CAPpHfdukVbJZntibZZ4HM7p92zN-QmAtD1+sAALRTFCsvpAq7A@mail.gmail.com

------
Regards,
Alexander Korotkov
Supabase

Hi!

On Sun, Sep 29, 2024 at 1:40 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:

Thank you for your review.

On Fri, Sep 27, 2024 at 7:35 AM Michael Paquier <michael@paquier.xyz> wrote:

On Fri, Sep 20, 2024 at 03:00:20PM +0300, Alexander Korotkov wrote:

Please, check the attached patchset for implementation of proposed approach.

0001 looks like it requires an indentation in its .h diffs.

+typedef enum
+{
+       WaitLSNResultSuccess, /* Target LSN is reached */
+       WaitLSNResultTimeout, /* Timeout occured */

Perhaps use WAIT_LSN_RESULT_SUCCESS, etc, rather than camel casing.

+ * Results statuses for WaitForLSNReplay().

Too much plural here.

What you are doing with WaitForLSNReplay() sounds kind of right.

rc = WaitLatch(MyLatch, wake_events, delay_ms,
WAIT_EVENT_WAIT_FOR_WAL_REPLAY);

Thank you, fixed in the attached patchset.

Question about this existing bit in waitlsn.c. Shouldn't this issue a
FATAL if rc reports a WL_POSTMASTER_DEATH? Or am I missing an
intention here. That was already the case before this patch set.

Fixed in the separate patch.

pg_wal_replay_wait() is new to v18, meaning that we still have some
time to agree on its final shape for this release cycle. This
discussion shares similarities with the recent exercise done in
f593c5517d14, and I think that we should be more consistent between
both to not repeat the same mistakes as in the past, even if this case
if more complex because we have more reasons behind why a wait could
not have happened.

I would suggest to keep things simple and have one single function
rather than introduce two more pg_proc entries with slight differences
in their error reporting, making the original function return a text
about the reason of the failure when waiting (be it a timeout,
success, promotion or outside recovery).

I also like to keep things simple. Keeping this as a single function
is not possible due to the reasons I described in [1]. However, it's
possible to fit into one stored procedure. I made 'no_error' as an
argument for the pg_wal_replay_wait() procedure. Done so in the
attached patchset.

FWIW, I am confused regarding the need for WaitLSNResultNotInRecovery
and WaitLSNResultPromotedConcurrently in the error states. On a code
basis, they check the same thing: RecoveryInProgress(). I'd suggest
to cut one of them.

Agreed. I initially intended to distinguish situations when the user
mistakenly calls pg_wal_replay_wait() on replication leader and when
concurrent promotion happens. However, given that the promotion could
happen after the user issued the query and before waiting starts, it
doesn't make much sense.

This also points to the fact that
WaitForLSNReplay() has some duplication that's not required. We could
have less GetXLogReplayRecPtr() calls and do all the checks in the for
loop. The current structure of the code in waitlsn.c is more complex
than it could be.

Not sure about this. I'd like to keep the fast-path check before we
call addLSNWaiter().

Please, check the revised patchset. It contains more tests for new
function pg_wal_replay_wait_status() and changes for documentation.

------
Regards,
Alexander Korotkov
Supabase

On 02.09.24 01:55, Alexander Korotkov wrote:

On Mon, Sep 2, 2024 at 2:28 AM Michael Paquier <michael@paquier.xyz> wrote:

On Sun, Sep 01, 2024 at 10:35:27PM +0300, Alexander Korotkov wrote:

This path hasn't changes since the patch revision when it was a
utility command. I agree that this doesn't look like proper path for
stored procedure. But I don't think src/backend/utils/adt is
appropriate path either, because it's not really about data type.
pg_wal_replay_wait() looks a good neighbor for
pg_wal_replay_pause()/pg_wal_replay_resume() and other WAL-related
functions. So, what about moving it to src/backend/access/transam?

Moving the new function to xlogfuncs.c while publishing
WaitForLSNReplay() makes sense to me.

Thank you for proposal. I like this.

Could you, please, check the attached patch?

We still have stuff in src/backend/commands/waitlsn.c that is nothing
like a "command". You have moved some stuff elsewhere, but what are you
planning to do with the rest?

On Wed, Oct 16, 2024 at 10:35 PM Peter Eisentraut <peter@eisentraut.org> wrote:

On 02.09.24 01:55, Alexander Korotkov wrote:

On Mon, Sep 2, 2024 at 2:28 AM Michael Paquier <michael@paquier.xyz> wrote:

On Sun, Sep 01, 2024 at 10:35:27PM +0300, Alexander Korotkov wrote:

This path hasn't changes since the patch revision when it was a
utility command. I agree that this doesn't look like proper path for
stored procedure. But I don't think src/backend/utils/adt is
appropriate path either, because it's not really about data type.
pg_wal_replay_wait() looks a good neighbor for
pg_wal_replay_pause()/pg_wal_replay_resume() and other WAL-related
functions. So, what about moving it to src/backend/access/transam?

Moving the new function to xlogfuncs.c while publishing
WaitForLSNReplay() makes sense to me.

Thank you for proposal. I like this.

Could you, please, check the attached patch?

We still have stuff in src/backend/commands/waitlsn.c that is nothing
like a "command". You have moved some stuff elsewhere, but what are you
planning to do with the rest?

Thank you for spotting this another time. What about moving that
somewhere like src/backend/access/transam/xlogwait.c ?

------
Regards,
Alexander Korotkov
Supabase