Re: pgsql: Implement pg_wal_replay_wait() stored procedure

Heikki Linnakangas <hlinnaka@iki.fi>

From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Alexander Korotkov <aekorotkov@gmail.com>, Michael Paquier <michael@paquier.xyz>, Pavel Borisov <pashkin.elfe@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2024-11-04T17:11:58Z
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 04/11/2024 18:53, Robert Haas wrote:
> On Mon, Nov 4, 2024 at 10:57 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>> Maybe that's just a better fit and we don't need either a procedure
>>> or new syntax.
>> I think it would still be good to expose the feature at SQL level too.
>> Makes it easier to test and makes it usable without client library
>> changes, for example.
> 
> OK. Well then I agree with Álvaro. If using a procedure isn't working
> out nicely, it's better to consider alternatives than to force a round
> peg into a square hole (with due regard for the fact that you can fit
> anything through the square hole if you make the square hole big
> enough...).

Agreed. Off the top of my, summary of issues I'm aware of:

- Missing documentation. The function itself is documented, but it's 
clearly intended for implementing read-after-write consistency in a 
cluster setup, so we should document how to do that.
- Not clear to me what the use case for the 'timeout' and 'noerror' 
arguments is. Why do we need it at all? (See first point on missing docs).
- Separate pg_wal_replay_wait_status() function to get result status is 
awkward
- Inconsistencies in behavior if server is not in recovery (Alexander's 
latest patch addresses this, but I didn't review it yet)
- pg_wal_replay_wait_status() is incorrectly marked as STABLE and 
PARALLEL SAFE
- Broken with default_transaction_isolation ='repeatable read'

Let's revert commit e546989a26 and rethink. I think the basic 
pg_wal_replay_wait() interface before e546989a26 was fine. Some of the 
above are issues above were present before e546989a26 already and need 
to be addressed in any case, but I believe they can be fixed without 
changing the interface.

-- 
Heikki Linnakangas
Neon (https://neon.tech)