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
