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)
