Horton

Re: pgsql: Implement pg_wal_replay_wait() stored procedure

Michael Paquier <michael@paquier.xyz>

View thread
From: Michael Paquier <michael@paquier.xyz>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Peter Eisentraut <peter@eisentraut.org>, Alexander Korotkov <akorotkov@postgresql.org>, pgsql-committers@lists.postgresql.org
Date: 2024-09-02T00:24:55Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Revert pg_wal_replay_wait() stored procedure

  2. Add 'no_error' argument to pg_wal_replay_wait()

  3. Refactor WaitForLSNReplay() to return the result of waiting

  4. Make WaitForLSNReplay() issue FATAL on postmaster death

  5. Move LSN waiting declarations and definitions to better place

  6. Update oid for pg_wal_replay_wait() procedure

  7. Move pg_wal_replay_wait() to xlogfuncs.c

  8. Implement pg_wal_replay_wait() stored procedure

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