Horton

Re: Implement waiting for wal lsn replay: reloaded

Yura Sokolov <y.sokolov@postgrespro.ru>

View thread
From: Yura Sokolov <y.sokolov@postgrespro.ru>
To: Alexander Korotkov <aekorotkov@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-02-06T08:31:28Z
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. Clean up 019_replslot_limit.pl comments

  2. Stabilize 019_replslot_limit.pl: wait on slot restart_lsn

  3. Use WAIT FOR LSN in PostgreSQL::Test::Cluster::wait_for_catchup()

  4. Add tab completion for the WAIT FOR LSN MODE option

  5. Add the MODE option to the WAIT FOR LSN command

  6. Extend xlogwait infrastructure with write and flush wait types

  7. Unify error messages

  8. Optimize shared memory usage for WaitLSNProcInfo

  9. Fix WaitLSNWakeup() fast-path check for InvalidXLogRecPtr

  10. Fix incorrect function name in comments

  11. Add infrastructure for efficient LSN waiting

  12. Add pairingheap_initialize() for shared memory usage

  13. Implement WAIT FOR command

27.11.2024 07:08, Alexander Korotkov wrote:
> Present solution
> 
> The present patch implements a new utility command WAIT FOR LSN
> 'target_lsn' [, TIMEOUT 'timeout'][, THROW 'throw'].  Unlike previous
> attempts to implement custom syntax, it uses only one extra unreserved
> keyword.  The parameters are implemented as generic_option_list.
> 
> Custom syntax eliminates the problem of running within an empty
> transaction of REPEATABLE READ level or higher.  We don't need to
> lookup a system catalog.  Thus, we have to set a transaction snapshot.
> 
> Also, revising PlannedStmtRequiresSnapshot() allows us to avoid
> holding a snapshot to return a value.  Therefore, the WAIT command in
> the attached patch returns its result status.
> 
> Also, the attached patch explicitly checks if the standby has been
> promoted to throw the most relevant form of an error.  The issue of
> inaccurate error messages has been previously spotted in [5].
> 
> Any comments?

Good day, Alexander.

I briefly looked into patch and have couple of minor remarks:

1. I don't like `palloc` in the `WaitLSNWakeup`. I believe it wont issue
problems, but still don't like it. I'd prefer to see local fixed array, say
of 16 elements, and loop around remaining function body acting in batch of
16 wakeups. Doubtfully there will be more than 16 waiting clients often,
and even then it wont be much heavier than fetching all at once.

2. I'd move `inHeap` field between `procno` and `phNode` to fill the gap
between fields on 64bit platforms.
Well, I believe, it would be better to tweak `pairingheap_node` to make it
clear if it is in heap or not. But such change would be unrelated to
current patch's sense. So lets stick with `inHeap`, but move it a bit.

Non-code question: do you imagine for `WAIT` command reuse for other cases?
Is syntax rule in gram.y convenient enough for such reuse? I believe, `LSN`
is not part of syntax to not introduce new keyword. But is it correct way?
I have no answer or strong opinion.

Otherwise, the patch looks quite strong to me.

-------
regards
Yura Sokolov