Hi, Ivan!

On Fri, Jun 30, 2023 at 11:32 AM Картышов Иван <i.kartys...@postgrespro.ru>
wrote:

> All rebased and tested
>

Thank you for continuing to work on this patch.

I see you're concentrating on the procedural version of this feature.  But
when you're calling a procedure within a normal SQL statement, the executor
gets a snapshot and holds it until the procedure finishes. In the case the
WAL record conflicts with this snapshot, the query will be canceled.
Alternatively, when hot_standby_feedback = on, the query and WAL replayer
will be in a deadlock (WAL replayer will wait for the query to finish, and
the query will wait for WAL replayed).  Do you see this issue?  Or do you
think I'm missing something?

XLogRecPtr
GetMinWaitedLSN(void)
{
    return state->min_lsn.value;
}

You definitely shouldn't access directly the fields
inside pg_atomic_uint64.  In this particular case, you should
use pg_atomic_read_u64().

Also, I think there is a race condition.

    /* Check if we already reached the needed LSN */
    if (cur_lsn >= target_lsn)
        return true;

    AddWaitedLSN(target_lsn);

Imagine, PerformWalRecovery() will replay a record after the check, but
before AddWaitedLSN().  This code will start the waiting cycle even if the
LSN is already achieved.  Surely this cycle will end soon because it
rechecks LSN value each 100 ms.  But anyway, I think there should be
another check after AddWaitedLSN() for the sake of consistency.

------
Regards,
Alexander Korotkov

Reply via email to