Message-ID: <14de8671-e328-4c3e-b136-664f6f13a39f@iki.fi>
Date: Mon, 28 Oct 2024 11:36:43 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: pgsql: Implement pg_wal_replay_wait() stored procedure
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Pavel Borisov <pashkin.elfe@gmail.com>,
 Peter Eisentraut <peter@eisentraut.org>,
 Michael Paquier <michael@paquier.xyz>, pgsql-committers@lists.postgresql.org
References: <E1sZwuz-002NPQ-Lc@gemulon.postgresql.org>
 <18c0fa64-0475-415e-a1bd-665d922c5201@eisentraut.org>
 <CAPpHfduQPYWATA2rfbYXsXM+Yspn1nQVwrEs=p303O428J57Yg@mail.gmail.com>
 <ZtT4dhlxuxdPPI8t@paquier.xyz>
 <CAPpHfdubNR746cbxRvs8C0MmNvUTtrjWwZ=RzcB5i_4kUz+RuQ@mail.gmail.com>
 <1f5b399a-76b1-48e1-87b0-f511e61818f3@eisentraut.org>
 <CAPpHfdvbcTBk_yuvvg=HvWJeLOWBtdmh1_K4j4btZ70ES_HWuw@mail.gmail.com>
 <CAPpHfdut8AvtU9PhXBOd0r5LPYkLyPeiWS+sfZ0FRoUBrS3_TQ@mail.gmail.com>
 <CALT9ZEFX_9CF1y5NFMDBVM2uACWUCgsKDfCS0dNMTQ=G+iFD5g@mail.gmail.com>
 <CAPpHfdu7eAN6_Xfs+xSnRhu1zoN6WiOvj58i6qv27FgYcd8DSg@mail.gmail.com>
 <CALT9ZEERNvqJ1iDk==G_B-ZZXUBZROaEptR-zUC4mHC+iUmHUQ@mail.gmail.com>
 <CAPpHfdtc5AG0bRDaDPX+3UfwnONauGs6=vXzSKSnNs_rY8dfqg@mail.gmail.com>
 <ab0eddce-06d4-4db2-87ce-46fa2427806c@iki.fi>
 <CAPpHfdu+9NnT8Mx8gpYVXy7-BHANDVdrD6qRwt=-1=m1_pWMxw@mail.gmail.com>
Content-Language: en-US
From: Heikki Linnakangas <hlinnaka@iki.fi>
In-Reply-To: 
 <CAPpHfdu+9NnT8Mx8gpYVXy7-BHANDVdrD6qRwt=-1=m1_pWMxw@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/14de8671-e328-4c3e-b136-664f6f13a39f%40iki.fi>
Precedence: bulk

On 25/10/2024 14:56, Alexander Korotkov wrote:
> I see that pg_wal_replay_wait_status() might look weird, but it seems
> to me like the best of feasible solutions. 

I haven't written many procedures, but our docs say:

 > Procedures do not return a function value; hence CREATE PROCEDURE 
lacks a RETURNS clause. However, procedures can instead return data to 
their callers via output parameters.

Did you consider using an output parameter?

> Given that
> pg_wal_replay_wait() procedure can't work concurrently to a query
> involving pg_wal_replay_wait_status() function, I think
> pg_wal_replay_wait_status() should be stable and parallel safe.

If you call pg_wal_replay_wait() in the backend process, and 
pg_wal_replay_wait_status() in a parallel worker process, it won't 
return the result of the wait. Probably not what you'd expect. So I'd 
argue that it should be parallel unsafe.

> This is the brief answer.  I will be able to come back with more
> details on Monday.

Thanks. A few more minor issues I spotted while playing with this:

- If you pass a very high value as the timeout, e.g. INT_MAX-1, it wraps 
around and doesn't wait at all
- You can pass NULLs as arguments. That should probably not be allowed, 
or we need to document what it means.

This is disappointing:

> postgres=# set default_transaction_isolation ='repeatable read';
> SET
> postgres=# call pg_wal_replay_wait('0/55DA24F');
> ERROR:  pg_wal_replay_wait() must be only called without an active or registered snapshot
> DETAIL:  Make sure pg_wal_replay_wait() isn't called within a transaction with an isolation level higher than READ COMMITTED, another procedure, or a function.

Is there any way we could make that work? Otherwise, the feature just 
basically doesn't work if you use repeatable read.

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